I was trying this problem where we need to find the permutations of the elements in the array.
This is the leetcode problem no 46. The issue I've faced is that I'm not able to output the ans
it just keeps returning a blank ArrayLists
:
Code:
public List<List<Integer>> permute(int[] nums)
{
List<List<Integer>> fans = new ArrayList<>();
HashMap<Integer, Integer> fmap = new HashMap<>();
for(int i: nums){
fmap.put(i, fmap.getOrDefault(i, 0) 1);
}
int n=nums.length;
List<Integer> ans=new ArrayList<>(n);
dfs(1, n, fmap, ans, fans);
return fans;
}
public void dfs(int cs, int ts, HashMap<Integer, Integer> fmap,List<Integer> ans, List<List<Integer>> fans)
{
if (cs > ts)
{
fans.add(ans);
return;
}
for(Integer val: fmap.keySet())
{
if (fmap.get(val) > 0)
{
fmap.put(val, fmap.get(val) - 1);
ans.add(val);
dfs(cs 1, ts, fmap, ans, fans);
ans.remove(ans.size() - 1);
fmap.put(val, fmap.get(val) 1);
}
}
}
Output for the test case [0,1]
:
[[],[]]
The actual output should be:
[[0,1],[1,0]]
When I'm checking the "potential answer" inside the recursive method, I am able to see the correct answer. I mean, if I print the output in the dfs
method, it shows the correct answer:
Change in the code:
if (cs > ts)
{
fans.add(ans);
System.out.println(fans);
return;
}
Now it's printing the value of fans
:
[[0, 1]]
[[1, 0], [1, 0]]
But these values are not being updated in the fans
and the returned value comes up blank.
I read someone mention this same issue, but it was for Python, and the solution in that case was to do a deep copy of the list.
I'm not sure how to do that in Java.
What I'm doing wrong?
CodePudding user response:
In order to generate a list of permutations, you don't need a Map
. You've only introduced redundant actions, which are not useful anyhow. If you doubt, add a couple of print-statements to visualize the map state it will always contain the same key with the value 1
(all numbers in the input are guaranteed to be unique) and it has no impact on the result.
Source of data for generating the Permutations
Besides the fact that attempt to utilize the HashMap
as the source of data for generating permutations isn't working because of the bugs, it's also not a good idea because the order iteration over the keySet of HashMap
is not guaranteed to be consistent.
As the uniform mean for storing the numbers that haven't yet been applied in current permutation, we can use an ArrayList
. In this case because there will be no duplicates in the input (see the quote from the leetcode below), we can use a LinkedHashSet
instead to improve performance. As explained below, a removal of elements will happen at before making every recursive a call, and removal from an ArrayList
has a cost of O(n), meanwhile with LinkedHashSet
it would be reduced to O(1).
Constraints:
1 <= nums.length <= 6
-10 <= nums[i] <= 10
All the integers of nums are unique.
Generating the Permutations
Each generated permutation should be contained in its own list. In your code, you've created one single list which is being passed around during recursive calls and eventually every recursive branch adds the same list to the resulting list. Which obviously should not happen.
You see, the result is being printed as [[],[]]
. It seems like a list containing two lists, but in fact they refer to the same empty list.
And this list is empty because every element that was added to it, is being removed after performing a recursive call:
ans.add(val);
... <- rursive call in between
ans.remove(ans.size() - 1); // removes the last element
if I print the output in the dfs method, it shows the correct answer:
Actually, it's not correct. If you take a careful look at the results, you'll see the nested lists are the same [[1, 0], [1, 0]]
.
The final result is blank because all recursive calls are happening between each value being added and removed (see the code snippet above). I.e. removal will be performed in revered order. That would be the last lines to be executed, not the return
statements. To understand it better, I suggest you to walk through the code line by line and draw on paper all the changes done to the ans
list for a simple input like [0, 1]
.
Instead, you should create a copy of the list containing not fully generated permutation (answer
) and then add an element into the copy. So that the initial permutation (answer
) remains unaffected and can be used as a template in all subsequent iterations.
List<Integer> updatedAnswer = new ArrayList<>(answer);
updatedAnswer.add(next);
And you also need to create a copy of the source of data and remove the element added to the newly created permutation (answer) in order to avoid repeating this element:
Set<Integer> updatedSource = new LinkedHashSet<>(source);
updatedSource.remove(next);
Sidenote: it's a good practice to give meaningful names to methods and variables. For instance, names cs
and ts
aren't informative (it's not clear what they are meant to store without looking at the code), method-name dfs
is confusing, DFS is a well-known algorithm, which is used for traversing tree or graph data structures, but it's not related to this problem.
Building a recursive solution
It makes sense to keep the recursive method to be void
to avoid wrapping the result with an additional list that would be thrown away afterwards, but in general it's more handy to return the result rather than accumulating it in the parameter. For performance reasons, I'll keep the method to be void
.
Every recursive implementation should contain two parts:
- Base case - that represents a simple edge-case (or a set of edge-cases) for which the outcome is known in advance. For this problem the base case would represent a situation when the given permutation has reached the size of the initial array, i.e. the source will contain no element, and we need to check whether it's empty or not. Parameters
cs
andts
that were used for this check in the solution provided in the question are redundant. - Recursive case - a part of a solution where recursive calls a made and when the main logic resides. In the recursive case, we need to replicate the given answer and source as explained above and use the updated copies as the arguments for each recursive call.
That's how it might be implemented:
public static List<List<Integer>> permute(int[] nums) {
Set<Integer> source = new LinkedHashSet<>();
for (int next: nums) source.add(next);
List<List<Integer>> result = new ArrayList<>();
permute(source, new ArrayList<>(), result);
return result;
}
public static void permute(Set<Integer> source, List<Integer> answer,
List<List<Integer>> result) {
if (source.isEmpty()) {
result.add(answer);
return;
}
for (Integer next: source) {
List<Integer> updatedAnswer = new ArrayList<>(answer);
updatedAnswer.add(next);
Set<Integer> updatedSource = new LinkedHashSet<>(source);
updatedSource.remove(next);
permute(updatedSource, updatedAnswer, result);
}
}
main()
public static void main(String[] args) {
int[] source = {1, 2, 3};
List<List<Integer>> permutations = permute(source);
for (List<Integer> permutation: permutations) {
System.out.println(permutation);
}
}
Output:
[1, 2, 3]
[1, 3, 2]
[2, 1, 3]
[2, 3, 1]
[3, 1, 2]
[3, 2, 1]