Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: redesign ArrayCombination #5181

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package com.thealgorithms.backtracking;

import java.util.ArrayList;
import java.util.List;
import java.util.TreeSet;

/**
* Finds all permutations of 1...n of length k
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Permutations?

Expand All @@ -10,23 +10,35 @@
public final class ArrayCombination {
private ArrayCombination() {
}
private static int length;

/**
* Find all combinations of 1..n by creating an array and using backtracking in Combination.java
* @param n max value of the array.
* @param k length of combination
* @return a list of all combinations of length k. If k == 0, return null.
* Find all combinations of 1..n using backtracking.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wat do you think about

Suggested change
* Find all combinations of 1..n using backtracking.
* Find all combinations of 0..n-1 using backtracking.

Clearly this will require changes in the code and tests, but in my opinion it will be much more useful.

*
* @param n Max value of the elements.
* @param k Length of the combination.
* @return A list of all combinations of length k.
* Returns an empty list if k is 0 or n is less than k.
Comment on lines +19 to +20
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will need an update.

*/
public static List<TreeSet<Integer>> combination(int n, int k) {
if (n <= 0) {
return null;
public static List<List<Integer>> combination(int n, int k) {
if (k <= 0 || n < k) {
return new ArrayList<>(); // Return empty list for invalid input
}
Comment on lines +23 to 25
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is nothing bad with n >= 0 and k == 0. Please throw IllegalArgumentException if one of the inputs is negative or n < k.

Please add tests checking if the exception is thrown.

length = k;
Integer[] arr = new Integer[n];
for (int i = 1; i <= n; i++) {
arr[i - 1] = i;

List<List<Integer>> combinations = new ArrayList<>();
combine(combinations, new ArrayList<>(), 1, n, k);
return combinations;
}

private static void combine(List<List<Integer>> combinations, List<Integer> current, int start, int n, int k) {
if (current.size() == k) { // Base case: combination found
combinations.add(new ArrayList<>(current)); // Copy to avoid modification
return;
}

for (int i = start; i <= n; i++) {
current.add(i);
combine(combinations, current, i + 1, n, k); // Recursive call
current.remove(current.size() - 1); // Backtrack
Comment on lines +34 to +41
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These comments are not too useful.

}
return Combination.combination(arr, length);
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not to express these tests like that:

package com.thealgorithms.backtracking;

import static org.junit.jupiter.api.Assertions.assertEquals;

import java.util.ArrayList;
import java.util.List;
import java.util.stream.Stream;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

public class ArrayCombinationTest {
    @ParameterizedTest
    @MethodSource("tcStream")
    void testWithRegularInput(final int n, final int k, final List<List<Integer>> expected) {
        assertEquals(expected, ArrayCombination.combination(n, k));
    }

    private static Stream<Arguments> tcStream() {
        return Stream.of(Arguments.of(4, 0, new ArrayList<ArrayList<Integer>>()), Arguments.of(3, 1, List.of(List.of(1), List.of(2), List.of(3))), Arguments.of(4, 2, List.of(List.of(1, 2), List.of(1, 3), List.of(1, 4), List.of(2, 3), List.of(2, 4), List.of(3, 4))));
    }
}

Original file line number Diff line number Diff line change
@@ -1,51 +1,48 @@
package com.thealgorithms.backtracking;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;

import java.util.ArrayList;
import java.util.List;
import java.util.TreeSet;
import org.junit.jupiter.api.Test;

public class ArrayCombinationTest {

@Test
void testNBeingZeroOrLess() {
List<TreeSet<Integer>> zeroResult = ArrayCombination.combination(0, 1);
List<TreeSet<Integer>> negativeResult = ArrayCombination.combination(-1, 1);
assertNull(zeroResult);
assertNull(negativeResult);
public void testCombination() {
// Test case 1: n = 4, k = 2
List<List<Integer>> expected = new ArrayList<>();
expected.add(List.of(1, 2));
expected.add(List.of(1, 3));
expected.add(List.of(1, 4));
expected.add(List.of(2, 3));
expected.add(List.of(2, 4));
expected.add(List.of(3, 4));

List<List<Integer>> actual = ArrayCombination.combination(4, 2);

assertEquals(expected, actual);
}

@Test
void testNoLengthElement() {
List<TreeSet<Integer>> result = ArrayCombination.combination(2, 0);
assertNull(result);
}
public void testEmptyCombination() {
// Test case 2: n = 4, k = 0 (invalid input)
List<List<Integer>> expected = new ArrayList<>();
List<List<Integer>> actual = ArrayCombination.combination(4, 0);

@Test
void testLengthOne() {
List<TreeSet<Integer>> result = ArrayCombination.combination(2, 1);
assert result != null;
assertEquals(1, result.get(0).iterator().next());
assertEquals(2, result.get(1).iterator().next());
assertEquals(expected, actual);
}

@Test
void testLengthTwo() {
List<TreeSet<Integer>> result = ArrayCombination.combination(2, 2);
assert result != null;
Integer[] arr = result.get(0).toArray(new Integer[2]);
assertEquals(1, arr[0]);
assertEquals(2, arr[1]);
}
public void testSingleElementCombinations() {
// Test case 3: n = 3, k = 1
List<List<Integer>> expected = new ArrayList<>();
expected.add(List.of(1));
expected.add(List.of(2));
expected.add(List.of(3));

@Test
void testLengthFive() {
List<TreeSet<Integer>> result = ArrayCombination.combination(10, 5);
assert result != null;
Integer[] arr = result.get(0).toArray(new Integer[5]);
assertEquals(1, arr[0]);
assertEquals(5, arr[4]);
List<List<Integer>> actual = ArrayCombination.combination(3, 1);

assertEquals(expected, actual);
}
}