Created a Permutation Function in the Recursion package. Closed the issue #7322#7412
Created a Permutation Function in the Recursion package. Closed the issue #7322#7412trynafind-sumanyu wants to merge 8 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7412 +/- ##
============================================
+ Coverage 79.54% 79.55% +0.01%
- Complexity 7178 7186 +8
============================================
Files 798 799 +1
Lines 23467 23492 +25
Branches 4617 4621 +4
============================================
+ Hits 18666 18690 +24
- Misses 4055 4056 +1
Partials 746 746 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@trynafind-sumanyu there exists another permutation class: https://github.com/TheAlgorithms/Java/blob/master/src/main/java/com/thealgorithms/backtracking/Permutation.java |
|
Hey @DenizAltunkapan, I wasn’t aware that a permutation implementation already existed under the backtracking package, as the issue specifically referenced the recursion package, where no such file was present. Based on that, I proceeded with adding a new implementation there. Additionally, I introduced handling for duplicate elements to ensure that only unique permutations are generated. This behavior was not present in the existing backtracking/Permutation.java implementation. Please let me know if you’d prefer aligning this with the existing implementation or consolidating the changes. |
@trynafind-sumanyu You can align these in the existing implementation. Just make sure that there aren't any duplicates. Thank you and sorry for the late reply : ) |
prashantpiyush1111
left a comment
There was a problem hiding this comment.
Overall the implementation is well-structured with good
test coverage. However, the main concern is the duplicate
class — please align with the existing
backtracking/Permutation.java as suggested by
@DenizAltunkapan. Please address all inline comments. 🙌
| * @throws NullPointerException if items is null | ||
| * @throws IllegalArgumentException if any element in items is null | ||
| */ | ||
| public static <T> List<List<T>> permutations(T[] items) { |
There was a problem hiding this comment.
As mentioned by @DenizAltunkapan, a Permutation class already
exists at backtracking/Permutation.java. Please align your
unique permutations logic into the existing implementation
instead of creating a separate class.
| return; | ||
| } | ||
|
|
||
| Set<T> seen = new HashSet<>(); |
There was a problem hiding this comment.
HashSet relies on hashCode() and equals() for deduplication.
For custom objects that don't override these methods,
duplicates may not be detected correctly.
Consider documenting this assumption in JavaDoc or
using explicit comparison instead.
| import java.util.List; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| class PermutationsTest { |
There was a problem hiding this comment.
Test file name should be PermutationsTest.java not
Permutations.java — as per TheAlgorithms naming convention.
| */ | ||
| public final class Permutations { | ||
|
|
||
| private Permutations() { |
There was a problem hiding this comment.
Private constructor is not tested — this is causing
2 uncovered lines in Codecov.
Add:
@test
void testConstructorThrowsException() {
assertThrows(UnsupportedOperationException.class, () -> {
var constructor = Permutations.class.getDeclaredConstructor();
constructor.setAccessible(true);
constructor.newInstance();
});
}
Issue #7322
clang-format -i --style=file path/to/your/file.java