The question is: Given n pairs of parentheses, write a function to generate all combinations of well-formed parentheses.
For example, given n = 3, a solution set is:
"((()))", "(()())", "(())()", "()(())", "()()()"
I used to solve this problem using string as following codes:
public class Solution {
public List<String> generateParenthesis(int n) {
ArrayList<String> result = new ArrayList<String>();
//StringBuilder s = new StringBuilder();
generate(n, 0, 0, "", result);
return result;
}
public void generate(int n, int left, int right, String s, ArrayList<String> result){
//left is num of '(' and right is num of ')'
if(left < right){
return;
}
if(left == n && right == n){
result.add(s);
return;
}
if(left == n){
//add ')' only.
generate(n, left, right + 1, s + ")", result);
return;
}
generate(n, left + 1, right, s + "(", result);
generate(n, left, right + 1, s + ")", result);
}
}
Now I want to solve this problem using StringBuilder, the code is like this:
import java.util.ArrayList;
public class generateParentheses {
public static ArrayList<String> generateParenthesis(int n) {
ArrayList<String> result = new ArrayList<String>();
StringBuilder sb = new StringBuilder();
generate(n, 0, 0, result, sb);
return result;
}
public static void generate(int n, int left, int right, ArrayList<String> result,
StringBuilder sb) {
if (left < right) {
return;
}
if (left == n && right == n) {
result.add(sb.toString());
sb = new StringBuilder();
return;
}
if (left == n) {
generate(n, left, right + 1, result, sb.append(')'));
return;
}
generate(n, left + 1, right, result, sb.append('('));
//sb.delete(sb.length(), sb.length() + 1);
generate(n, left, right + 1, result, sb.append(')'));
//sb.delete(sb.length(), sb.length() + 1);
}
public static void main(String[] args) {
// TODO Auto-generated method stub
System.out.println(generateParenthesis(4));
}
}
The result is not what I want: (((()))), (((()))))())), (((()))))())))()), (((()))))())))()))(), (((()))))())))()))()))(())), (((()))))())))()))()))(())))()).........
Is there anyone tell me what is the problem? Thank you very much.
The combination of a parameter, assigment and return:
does not make sense, because the parameter is passed in by value, i.e., sb is a local variable the change of which has no effect in the calling environment.
If you want to clear the StringBuilder, there is method sb.delete( start, end ) that will truly affect the object referenced via sb.
Your code is inefficient. Here is a better approach. We make an effective brute force over the 2^N possible bracket permutations with heavy pruning(we quit the recursion immediately if there is no possible valid solution for the current parameters). Here's the code in C++:
Some clarifications: pos marks the current position in the bracket solution we are generating. If we reach position N (that means the solution is valid) we have a valid solution in the
string result
variable and we simple push it in the vector with the valid solutions. Now for what we use balance you may ask. We make the observation that in order a bracket permutation to be valid, at any time at any given position the count of the '(' from the start must be greater than the count of the ')'s and with balance we measure their difference, so me put a ')' at a given position only if the balance > 0.Another much simpler way of doing this. Here, i am trying for a recursive solution where the recursive function adds balanced parentheses in 3 ways: "()"+result and result+"()" and "("+result+")" for each item in HashSet returned by the function it calls then put them in a HashSet to remove duplicates.
After carefully debuting this program, I found out the problem. The correct code is listed as follows:
And the result is: (((()))), ((()())), ((())()), ((()))(), (()(())), (()()()), (()())(), (())(()), (())()(), ()((())), ()(()()), ()(())(), ()()(()), ()()()()
You ware close. Your mistakes ware:
sb
instead of removing only its last characterway you want to reset
sb
:sb = new StringBuilder();
you are reassigningsb
which is local variable of current method, not variable of method which invoked it (Java is not pass-by-reference but pass-by-value).your almost correct attempt ware commented
sb.delete(sb.length(), sb.length() + 1);
but here you are actually trying to remove characters starting from positionsb.length()
, but just like arrays indexes of character in StringBuilder are from0
tillsb.length() - 1
so is trying to remove one character after last character which effectively can't remove anything.What you needed here is
or more readable
but probably best approach in terms of performance
setLength
(described at bottom of answer)your logic of when to remove characters from StringBuilder is also flawed
you are doing it only in one place which ends (backtracks) recursive calls: after finding correct results. But what about other cases like
if (left < right)
or most importantly, if method will end normally likeHere
generate(3, 1, 2, ')');
ends and removes last character fromsb
, but shouldn't previous methodgenerate(3, 1, 1, ')')
also remove its own)
added to StringBuilder?In other words you shouldn't remove last character only at end of successful condition in recursive call, but after each recursive call, to make sure that method will also remove character it adds.
So change your code to something like
or try writing something probably more readable like
but while invoking you would need to set
maxLength
as2*n
since it is the max length StringBuilder should contain, so you would also have to changegenerateParenthesis(int n)
to:Farther improvement:
If you are aiming for performance then you probably don't want to use
delete
ordeleteCharAt
because each time it creates new array and fills it with copy of values from without ones you don't want.Instead you can use
setLength
method. If you will pass value which is smaller than number of currently stored characters it will setcount
to value passed in this method, which will effectively make characters after them irrelevant. In other words this characters will be no longer used in for instancetoString()
even if they will be still in StringBuilder buffer array.Example:
In line 1
StringBuilder
will allocate array for at least3
characters (by default it usesstr.length() + 16
to determine size of buffered array of characters it will store for now) and will place there characters from passed String, so it will containIndex of position where next character should be placed is stored in
count
field and for now it is equal to3
.In line 2 value of
count
will be set to2
, but our array will not be changed so it will still look likeIn line 3 new String will be created and printed, but it will be filled only with characters placed before index stored in
count
, which means that it will contain onlya
andb
(array will still be unchanged).In line 4 you will add new character to buffer and it will be placed after "important" characters. Since number of important characters is stored in
count
field (and they are placed at beginning of array), next irrelevant character must be at position pointed bycount
, which meansd
will be placed at position with index2
which means now array will look likeand value of
count
will be incremented (we added only one character socount
will now become3
).3
characters from array used byStringBuilder
so we will seeabd
.