ronantakizawa/codereview-bench huggingface dataset:
Repository: TheAlgorithms/Java
File: src/main/java/com/thealgorithms/dynamicprogramming/LongestArithmeticSubsequence.java (Java)

Code before:
final class LongestArithmeticSubsequence {

    private LongestArithmeticSubsequence() {
    }

    /**
     * Returns the length of the longest arithmetic subsequence in the given array.
     *
     * A sequence seq is arithmetic if seq[i + 1] - seq[i] are all the same value
     * (for 0 <= i < seq.length - 1).
     *
     * @param nums the input array of integers
     * @return the length of the longest arithmetic subsequence
     */
    public static int getLongestArithmeticSubsequenceLength(int[] nums) {
        if (nums == null) {
            throw new IllegalArgumentException("Input array cannot be null");
        }

        if (nums.length == 0) {
            return 0;
        }

        if (nums.length == 1) {
            return 1;
        }

        HashMap<Integer, Integer>[] dp = new HashMap[nums.length];
        int maxLength = 2;

        // Initialize dp array
        for (int i = 0; i < nums.length; i++) {
            dp[i] = new HashMap<>();
            for (int j = 0; j < i; j++) {
                final int diff = nums[i] - nums[j];
                dp[i].put(diff, dp[j].getOrDefault(diff, 1) + 1);
                maxLength = Math.max(maxLength, dp[i].get(diff));
            }
        }

        return maxLength;
    }
}


Review comment (suggestion):
```suggestion
        if (nums.length <= 1) {
            return nums.length;
        }
```

Code after:
final class LongestArithmeticSubsequence {
    private LongestArithmeticSubsequence() {
    }

    /**
     * Returns the length of the longest arithmetic subsequence in the given array.
     *
     * A sequence seq is arithmetic if seq[i + 1] - seq[i] are all the same value
     * (for 0 <= i < seq.length - 1).
     *
     * @param nums the input array of integers
     * @return the length of the longest arithmetic subsequence
     */
    public static int getLongestArithmeticSubsequenceLength(int[] nums) {
        if (nums == null) {
            throw new IllegalArgumentException("Input array cannot be null");
        }

        if (nums.length <= 1) {
            return nums.length;
        }

        HashMap<Integer, Integer>[] dp = new HashMap[nums.length];
        int maxLength = 2;

        // fill the dp array
        for (int i = 0; i < nums.length; i++) {
            dp[i] = new HashMap<>();
            for (int j = 0; j < i; j++) {
                final int diff = nums[i] - nums[j];
                dp[i].put(diff, dp[j].getOrDefault(diff, 1) + 1);
                maxLength = Math.max(maxLength, dp[i].get(diff));
            }
        }

        return maxLength;
    }
}
Repository: TheAlgorithms/Java
File: src/main/java/com/thealgorithms/dynamicprogramming/LongestArithmeticSubsequence.java (Java)

Code before:
    /**
     * Returns the length of the longest arithmetic subsequence in the given array.
     *
     * A sequence seq is arithmetic if seq[i + 1] - seq[i] are all the same value
     * (for 0 <= i < seq.length - 1).
     *
     * @param nums the input array of integers
     * @return the length of the longest arithmetic subsequence
     */
    public static int getLongestArithmeticSubsequenceLength(int[] nums) {
        if (nums == null) {
            throw new IllegalArgumentException("Input array cannot be null");
        }

        if (nums.length == 0) {
            return 0;
        }

        if (nums.length == 1) {
            return 1;
        }

        HashMap<Integer, Integer>[] dp = new HashMap[nums.length];
        int maxLength = 2;

        // Initialize dp array
        for (int i = 0; i < nums.length; i++) {
            dp[i] = new HashMap<>();
            for (int j = 0; j < i; j++) {
                final int diff = nums[i] - nums[j];
                dp[i].put(diff, dp[j].getOrDefault(diff, 1) + 1);
                maxLength = Math.max(maxLength, dp[i].get(diff));
            }
        }

        return maxLength;
    }
}


Review comment (suggestion):
I would remove it. Of:
```suggestion
        // fill the dp array
```

Code after:
     * Returns the length of the longest arithmetic subsequence in the given array.
     *
     * A sequence seq is arithmetic if seq[i + 1] - seq[i] are all the same value
     * (for 0 <= i < seq.length - 1).
     *
     * @param nums the input array of integers
     * @return the length of the longest arithmetic subsequence
     */
    public static int getLongestArithmeticSubsequenceLength(int[] nums) {
        if (nums == null) {
            throw new IllegalArgumentException("Input array cannot be null");
        }

        if (nums.length <= 1) {
            return nums.length;
        }

        HashMap<Integer, Integer>[] dp = new HashMap[nums.length];
        int maxLength = 2;

        // fill the dp array
        for (int i = 0; i < nums.length; i++) {
            dp[i] = new HashMap<>();
            for (int j = 0; j < i; j++) {
                final int diff = nums[i] - nums[j];
                dp[i].put(diff, dp[j].getOrDefault(diff, 1) + 1);
                maxLength = Math.max(maxLength, dp[i].get(diff));
            }
        }

        return maxLength;
    }
}
Repository: TheAlgorithms/Java
File: src/test/java/com/thealgorithms/Recursion/GenerateSubsetsTest.java (Java)

Code before:
package com.thealgorithms.Recursion;

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

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

public final class GenerateSubsetsTest {

    @Test
    void subsetRecursionTestOne() {
        String str = "abc";
        String[] expected = new String[] {"abc", "ab", "ac", "a", "bc", "b", "c", ""};

        List<String> ans = GenerateSubsets.subsetRecursion("", str);
        assertArrayEquals(ans.toArray(), expected);
    }

    @Test
    void subsetRecursionTestTwo() {
        String str = "cbf";
        String[] expected = new String[] {"cbf", "cb", "cf", "c", "bf", "b", "f", ""};

        List<String> ans = GenerateSubsets.subsetRecursion("", str);
        assertArrayEquals(ans.toArray(), expected);
    }

    @Test
    void subsetRecursionTestThree() {
        String str = "aba";
        String[] expected = new String[] {"aba", "ab", "aa", "a", "ba", "b", "a", ""};

        List<String> ans = GenerateSubsets.subsetRecursion("", str);
        assertArrayEquals(ans.toArray(), expected);
    }
}


Review comment (suggestion):
You always pass "" as the first parameter, which is not needed by the function. Create a facade function that will only have 1 parameter, and that will call the existing function with 2 parameters. e.g. `subsetRecursion(String str)` calls `doRecursionStep(String constructedString, String availableCharacters)`

Code after:
package com.thealgorithms.Recursion;

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

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

public final class GenerateSubsetsTest {

    @Test
    void subsetRecursionTestOne() {
        String str = "abc";
        String[] expected = new String[] {"abc", "ab", "ac", "a", "bc", "b", "c", ""};

        List<String> ans = GenerateSubsets.subsetRecursion(str);
        assertArrayEquals(ans.toArray(), expected);
    }

    @Test
    void subsetRecursionTestTwo() {
        String str = "cbf";
        String[] expected = new String[] {"cbf", "cb", "cf", "c", "bf", "b", "f", ""};

        List<String> ans = GenerateSubsets.subsetRecursion(str);
        assertArrayEquals(ans.toArray(), expected);
    }

    @Test
    void subsetRecursionTestThree() {
        String str = "aba";
        String[] expected = new String[] {"aba", "ab", "aa", "a", "ba", "b", "a", ""};

        List<String> ans = GenerateSubsets.subsetRecursion(str);
        assertArrayEquals(ans.toArray(), expected);
    }
}
Repository: TheAlgorithms/Java
File: src/main/java/com/thealgorithms/stacks/PrefixToInfix.java (Java)

Code before:
package com.thealgorithms.stacks;

import java.util.Stack;

/**
 * Prefix to Infix implementation via Stack
 *
 * Function: String getPrefixToInfix(String prefix)
 * Returns the Infix Expression for the given prefix parameter.
 *
 * Avoid using parentheses/brackets/braces for the prefix string.
 * Prefix Expressions don't require these.
 */

public final class PrefixToInfix {
    private PrefixToInfix() {
    }

    /**
     * Determines if a given character is a valid arithmetic operator.
     *
     * @param token the character to check
     * @return true if the character is an operator, false otherwise
     */
    public static boolean isOperator(char token) {
        return token == '+' || token == '-' || token == '/' || token == '*' || token == '^';
    }

    /**
     * Converts a valid prefix expression to an infix expression.
     *

Review comment (suggestion):
How do you think this description could be changed to the following:

```
/**
 * Converts a prefix expression to an infix expression using a stack.
 * 
 * The input prefix expression should consist of valid operands (letters or digits)
 * and operators (+, -, *, /, ^). Parentheses are not required in the prefix string.
 */
```

Code after:
package com.thealgorithms.stacks;

import java.util.Stack;

/**
 * Converts a prefix expression to an infix expression using a stack.
 *
 * The input prefix expression should consist of
 * valid operands (letters or digits) and operators (+, -, *, /, ^).
 * Parentheses are not required in the prefix string.
 */
public final class PrefixToInfix {
    private PrefixToInfix() {
    }

    /**
     * Determines if a given character is a valid arithmetic operator.
     *
     * @param token the character to check
     * @return true if the character is an operator, false otherwise
     */
    public static boolean isOperator(char token) {
        return token == '+' || token == '-' || token == '/' || token == '*' || token == '^';
    }

    /**
     * Converts a valid prefix expression to an infix expression.
     *
     * @param prefix the prefix expression to convert
     * @return the equivalent infix expression
     * @throws NullPointerException     if the prefix expression is null
Repository: TheAlgorithms/Java
File: src/main/java/com/thealgorithms/stacks/PrefixToInfix.java (Java)

Code before:
        }

        Stack<String> stack = new Stack<>();

        // Iterate over the prefix expression from right to left
        for (int i = prefix.length() - 1; i >= 0; i--) {
            char token = prefix.charAt(i);

            if (isOperator(token)) {
                // Pop two operands from stack
                String operandA = stack.pop();
                String operandB = stack.pop();

                // Form the infix expression with parentheses
                String infix = "(" + operandA + token + operandB + ")";

                // Push the resulting infix expression back onto the stack
                stack.push(infix);
            } else {
                // Push operand onto stack
                stack.push(Character.toString(token));
            }
        }

        return stack.pop(); // final element on the stack is the full infix expression
    }
}


Review comment (suggestion):
As far as I understand, there should only be one element in the stack. Can you imagine a situation where the final stack will not have any elements, which would mean that the expression is not well formed?

Do you think it is worth adding such a constraint before return:

```
if (stack.size() != 1) {
    throw new ArithmeticException("Malformed prefix expression");
}
```

Code after:

        // Iterate over the prefix expression from right to left
        for (int i = prefix.length() - 1; i >= 0; i--) {
            char token = prefix.charAt(i);

            if (isOperator(token)) {
                // Pop two operands from stack
                String operandA = stack.pop();
                String operandB = stack.pop();

                // Form the infix expression with parentheses
                String infix = "(" + operandA + token + operandB + ")";

                // Push the resulting infix expression back onto the stack
                stack.push(infix);
            } else {
                // Push operand onto stack
                stack.push(Character.toString(token));
            }
        }

        if (stack.size() != 1) {
            throw new ArithmeticException("Malformed prefix expression");
        }

        return stack.pop(); // final element on the stack is the full infix expression
    }
}
Repository: TheAlgorithms/Java
File: src/test/java/com/thealgorithms/dynamicprogramming/UniqueSubsequencesCountTest.java (Java)

Code before:
package com.thealgorithms.dynamicprogramming;

import static org.junit.jupiter.api.Assertions.assertEquals;

import org.junit.jupiter.api.Test;

public final class UniqueSubsequencesCountTest {

    @Test
    void subseqCountTestOne() {
        String s = "abc";

        assertEquals(7, UniqueSubsequencesCount.subseqCount(s));
    }

    @Test
    void subseqCountTestTwo() {
        String s = "abcdashgdhas";

        assertEquals(3592, UniqueSubsequencesCount.subseqCount(s));
    }

    @Test
    void subseqCountTestThree() {
        String s = "aaaaa";

        assertEquals(5, UniqueSubsequencesCount.subseqCount(s));
    }
}


Review comment (refactor):
Better to refactor using parametrized tests (a lot of examples already exist in project).

And will be good to have more test cases. Possible testcases:

```
Empty String:
Should return 0 when the input string is empty.

Single Character:
A string with only one character should have one subsequence (the character itself).
"a"

Two Different Characters:
For a string with two different characters, there should be 3 subsequences: a, b, and ab.
"ab"

String with Repeated Characters:
A string with repeated characters should handle duplicates correctly.
"aaa"

All Unique Characters:
A string with all unique characters should have the maximum number of subsequences.
"abc"

String with Mixed Characters:
A string with a mix of repeated and unique characters.
"abab"

A string containing non-alphabetical characters like numbers or punctuation.
"a1b2"

Long String with Repeated Characters:
Tests ability to handle long input strings with many repeated characters. (But looks like such test already exist)
"aaaaaa"

String with Spaces:
Ensure that spaces are treated like any other character in the subsequence count.
"a b"

String with Upper and Lower Case:
A string containing both uppercase and lowercase characters to test case sensitivity.
"AaBb"

```

Code after:
package com.thealgorithms.dynamicprogramming;

import static org.junit.jupiter.api.Assertions.assertEquals;

import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;

public class UniqueSubsequencesCountTest {

    @ParameterizedTest
    @CsvSource({"abc, 7", "abcdashgdhas, 3592", "a, 1", "'a b', 7", "a1b2, 15", "AaBb, 15", "abab, 11"})
    void subseqCountParameterizedTest(String input, int expected) {
        assertEquals(expected, UniqueSubsequencesCount.countSubseq(input));
    }
}
Repository: TheAlgorithms/Java
File: src/main/java/com/thealgorithms/dynamicprogramming/SubsetSum.java (Java)

Code before:
     *
     * @param arr the array containing integers.
     * @param sum the target sum of the subset.
     * @return {@code true} if a subset exists that sums to the given value,
     *         otherwise {@code false}.
     */
    public static boolean subsetSum(int[] arr, int sum) {
        int n = arr.length;

        // Intialize Two Arrays to store current and prev states
        boolean[] isSumCurr = new boolean[sum + 1];
        boolean[] isSumPrev = new boolean[sum + 1];

        // Mark prev[0] = true as it is true to make sum = 0 using 0 elements
        isSumPrev[0] = true;

        // Fill the subset sum matrix
        for (int i = 1; i <= n; i++) {
            for (int j = 0; j <= sum; j++) {
                if (arr[i - 1] <= j) {
                    isSumCurr[j] = isSumPrev[j] || isSumPrev[j - arr[i - 1]];
                } else {
                    isSumCurr[j] = isSumPrev[j];
                }
            }
            isSumPrev = isSumCurr.clone();
        }

        return isSumPrev[sum];
    }
}


Review comment (performance):
The use of` isSumPrev = isSumCurr.clone();` inside the loop introduces unnecessary overhead. Cloning arrays in every iteration can be costly in terms of performance.

You can use two arrays and just swap their references, rather than cloning the array each time.

Code after:
     *
     * @param arr the array containing integers.
     * @param sum the target sum of the subset.
     * @return {@code true} if a subset exists that sums to the given value,
     *         otherwise {@code false}.
     */
    public static boolean subsetSum(int[] arr, int sum) {
        int n = arr.length;

        // Initialize a single array to store the possible sums
        boolean[] isSum = new boolean[sum + 1];

        // Mark isSum[0] = true since a sum of 0 is always possible with 0 elements
        isSum[0] = true;

        // Iterate through each Element in the array
        for (int i = 0; i < n; i++) {
            // Traverse the isSum array backwards to prevent overwriting values
            for (int j = sum; j >= arr[i]; j--) {
                isSum[j] = isSum[j] || isSum[j - arr[i]];
            }
        }
        return isSum[sum];
    }
}
Repository: TheAlgorithms/Java
File: src/main/java/com/thealgorithms/dynamicprogramming/SubsetSum.java (Java)

Code before:
package com.thealgorithms.dynamicprogramming;

public final class SubsetSum {
    private SubsetSum() {
    }

    /**
     * Test if a set of integers contains a subset that sums to a given integer.
     *
     * @param arr the array containing integers.
     * @param sum the target sum of the subset.
     * @return {@code true} if a subset exists that sums to the given value,
     *         otherwise {@code false}.
     */
    public static boolean subsetSum(int[] arr, int sum) {
        int n = arr.length;

        // Intialize Two Arrays to store current and prev states
        boolean[] isSumCurr = new boolean[sum + 1];
        boolean[] isSumPrev = new boolean[sum + 1];

        // Mark prev[0] = true as it is true to make sum = 0 using 0 elements
        isSumPrev[0] = true;

        // Fill the subset sum matrix
        for (int i = 1; i <= n; i++) {
            for (int j = 0; j <= sum; j++) {
                if (arr[i - 1] <= j) {
                    isSumCurr[j] = isSumPrev[j] || isSumPrev[j - arr[i - 1]];
                } else {
                    isSumCurr[j] = isSumPrev[j];
                }
            }
            isSumPrev = isSumCurr.clone();
        }

        return isSumPrev[sum];
    }
}


Review comment (performance):
While you're already using a more efficient space approach (two 1D arrays), it can be improved further by using only one array. You don't actually need both `isSumCurr `and `isSumPrev`. 

By processing the array in reverse order (starting from sum and decrementing down to 0), you can reuse a single array.

Code after:
package com.thealgorithms.dynamicprogramming;

public final class SubsetSum {
    private SubsetSum() {
    }

    /**
     * Test if a set of integers contains a subset that sums to a given integer.
     *
     * @param arr the array containing integers.
     * @param sum the target sum of the subset.
     * @return {@code true} if a subset exists that sums to the given value,
     *         otherwise {@code false}.
     */
    public static boolean subsetSum(int[] arr, int sum) {
        int n = arr.length;

        // Initialize a single array to store the possible sums
        boolean[] isSum = new boolean[sum + 1];

        // Mark isSum[0] = true since a sum of 0 is always possible with 0 elements
        isSum[0] = true;

        // Iterate through each Element in the array
        for (int i = 0; i < n; i++) {
            // Traverse the isSum array backwards to prevent overwriting values
            for (int j = sum; j >= arr[i]; j--) {
                isSum[j] = isSum[j] || isSum[j - arr[i]];
            }
        }
        return isSum[sum];
    }
}
Repository: TheAlgorithms/Java
File: src/test/java/com/thealgorithms/maths/ConvolutionTest.java (Java)

Code before:
package com.thealgorithms.maths;

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

import org.junit.jupiter.api.Test;

public class ConvolutionTest {

    @Test
    public void testConvolutionBasic() {
        double[] signalA = {1, 2, 3};
        double[] signalB = {4, 5, 6};
        double[] expected = {4, 13, 28, 27, 18}; // Expected output

        double[] result = Convolution.convolution(signalA, signalB);

        assertArrayEquals(expected, result, 1e-9); // Allowing for floating-point precision
    }

    @Test
    public void testConvolutionWithZeroElements() {
        double[] signalA = {0, 0, 0};
        double[] signalB = {1, 2, 3};
        double[] expected = {0, 0, 0, 0, 0}; // All values should remain zero

        double[] result = Convolution.convolution(signalA, signalB);

        assertArrayEquals(expected, result, 1e-9);
    }

    @Test
    public void testConvolutionSingleElement() {
        double[] signalA = {2};
        double[] signalB = {3};

Review comment (suggestion):
Will be better to improve these tests using `@ParameterizedTest` and using additional record or class with data for TestCase. There are a lot of examples in repository

Code after:
package com.thealgorithms.maths;

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

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

public class ConvolutionTest {

    record ConvolutionTestCase(String description, double[] signalA, double[] signalB, double[] expected) {
    }

    @ParameterizedTest(name = "{0}")
    @MethodSource("provideTestCases")
    void testConvolution(ConvolutionTestCase testCase) {
        double[] result = Convolution.convolution(testCase.signalA, testCase.signalB);
        assertArrayEquals(testCase.expected, result, 1e-9, testCase.description);
    }

    private static Stream<ConvolutionTestCase> provideTestCases() {
        return Stream.of(new ConvolutionTestCase("Basic convolution", new double[] {1, 2, 3}, new double[] {4, 5, 6}, new double[] {4, 13, 28, 27, 18}), new ConvolutionTestCase("Convolution with zero elements", new double[] {0, 0, 0}, new double[] {1, 2, 3}, new double[] {0, 0, 0, 0, 0}),
            new ConvolutionTestCase("Convolution with single element", new double[] {2}, new double[] {3}, new double[] {6}), new ConvolutionTestCase("Convolution with different sizes", new double[] {1, 2}, new double[] {3, 4, 5}, new double[] {3, 10, 13, 10}),
            new ConvolutionTestCase("Convolution with negative values", new double[] {1, -2, 3}, new double[] {-1, 2, -3}, new double[] {-1, 4, -10, 12, -9}),
            new ConvolutionTestCase("Convolution with large numbers", new double[] {1e6, 2e6}, new double[] {3e6, 4e6}, new double[] {3e12, 1e13, 8e12}));
    }
}