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

Code before:
        }
        return pigeonHoles;
    }

    /**
     * Populates the pigeonholes with elements from the array.
     *
     * @param array the array to be sorted
     * @param pigeonHoles the pigeonholes to be populated
     */
    private static void populatePigeonHoles(int[] array, List<List<Integer>> pigeonHoles) {
        for (int element : array) {
            pigeonHoles.get(element).add(element);
        }
    }

    /**
     * Collects sorted elements from the pigeonholes back into the array.
     *
     * @param array the array to be sorted
     * @param pigeonHoles the populated pigeonholes
     */
    private static void collectFromPigeonHoles(int[] array, List<List<Integer>> pigeonHoles) {
        int index = 0;
        for (List<Integer> pigeonHole : pigeonHoles) {
            for (int element : pigeonHole) {
                array[index++] = element;
            }
        }
    }
}


Review comment (suggestion):
```suggestion
        for (final var pigeonHole : pigeonHoles) {
            for (final int element : pigeonHole) {
```

Code after:
            pigeonHoles.add(new ArrayList<>());
        }
        return pigeonHoles;
    }

    /**
     * Populates the pigeonholes with elements from the array.
     *
     * @param array the array to be sorted
     * @param pigeonHoles the pigeonholes to be populated
     */
    private static void populatePigeonHoles(int[] array, List<List<Integer>> pigeonHoles) {
        for (int element : array) {
            pigeonHoles.get(element).add(element);
        }
    }

    /**
     * Collects sorted elements from the pigeonholes back into the array.
     *
     * @param array the array to be sorted
     * @param pigeonHoles the populated pigeonholes
     */
    private static void collectFromPigeonHoles(int[] array, List<List<Integer>> pigeonHoles) {
        int index = 0;
        for (final var pigeonHole : pigeonHoles) {
            for (final int element : pigeonHole) {
                array[index++] = element;
            }
        }
    }
}
Repository: TheAlgorithms/Java
File: src/main/java/com/thealgorithms/maths/Average.java (Java)

Code before:
    /**
     * Computes the average of a {@code double} array.
     *
     * @param numbers an array of {@code double} values
     * @return the average of the given numbers
     * @throws IllegalArgumentException if the input array is {@code null} or empty
     */
    public static double average(double[] numbers) {
        if (numbers == null || numbers.length == 0) {
            throw new IllegalArgumentException("Array must not be null or empty.");
        }
        double sum = 0;
        for (double number : numbers) {
            sum += number;
        }
        return sum / numbers.length;
    }

    /**
     * Computes the average of an {@code int} array.
     *
     * @param numbers an array of {@code int} values
     * @return the average of the given numbers
     * @throws IllegalArgumentException if the input array is {@code null} or empty
     */
    public static int average(int[] numbers) {
        if (numbers == null || numbers.length == 0) {
            throw new IllegalArgumentException("Array must not be null or empty.");
        }
        long sum = 0;
        for (int number : numbers) {
            sum += number;
        }
        return (int) (sum / numbers.length);
    }
}


Review comment (suggestion):
```suggestion
    public static double average(int[] numbers) {
```
Change the data type

Code after:
    /**
     * Computes the average of a {@code double} array.
     *
     * @param numbers an array of {@code double} values
     * @return the average of the given numbers
     * @throws IllegalArgumentException if the input array is {@code null} or empty
     */
    public static double average(double[] numbers) {
        if (numbers == null || numbers.length == 0) {
            throw new IllegalArgumentException("Numbers array cannot be empty or null");
        }
        double sum = 0;
        for (double number : numbers) {
            sum += number;
        }
        return sum / numbers.length;
    }

    /**
     * Computes the average of an {@code int} array.
     *
     * @param numbers an array of {@code int} values
     * @return the average of the given numbers
     * @throws IllegalArgumentException if the input array is {@code null} or empty
     */
    public static double average(int[] numbers) {
        if (numbers == null || numbers.length == 0) {
            throw new IllegalArgumentException("Numbers array cannot be empty or null");
        }
        long sum = 0;
        for (int number : numbers) {
            sum += number;
        }
        return (double) (sum / numbers.length);
    }
}
Repository: TheAlgorithms/Java
File: src/main/java/com/thealgorithms/geometry/GrahamScan.java (Java)

Code before:
        }

        int indexPoint2;
        for (indexPoint2 = indexPoint1 + 1; indexPoint2 < points.length; indexPoint2++) {
            if (Point.orientation(points[0], points[indexPoint1], points[indexPoint2]) != 0) {
                break;
            }
        }
        hull.push(points[indexPoint2 - 1]);

        // Process the remaining points and update the hull
        for (int i = indexPoint2; i < points.length; i++) {
            Point top = hull.pop();
            while (Point.orientation(hull.peek(), top, points[i]) <= 0) {
                top = hull.pop();
            }
            hull.push(top);
            hull.push(points[i]);
        }
    }

    /**
     * @return An iterable collection of points representing the convex hull.
     */
    public Iterable<Point> hull() {
        Stack<Point> result = new Stack<>();
        for (Point p : hull) {
            result.push(p);
        }
        return result;
    }

    public record Point(int x, int y) implements Comparable<Point> {

        /**
         * Default constructor
         * @param x x-coordinate
         * @param y y-coordinate
         */
        public Point {
        }

        /**
         * @return the x-coordinate
         */
        @Override
        public int x() {
            return x;
        }

        /**

Review comment (suggestion):
Is it important use here Stack? If we replace it via LinkedList for example, than we will have only:
`return new LinkedList<>(hull);`

Code after:
            }
        }

        if (firstNonEqualIndex == points.length) {
            return;
        }

        int firstNonCollinearIndex;
        for (firstNonCollinearIndex = firstNonEqualIndex + 1; firstNonCollinearIndex < points.length; firstNonCollinearIndex++) {
            if (Point.orientation(points[0], points[firstNonEqualIndex], points[firstNonCollinearIndex]) != 0) {
                break;
            }
        }

        hull.push(points[firstNonCollinearIndex - 1]);

        // Process the remaining points and update the hull
        for (int i = firstNonCollinearIndex; i < points.length; i++) {
            Point top = hull.pop();
            while (Point.orientation(hull.peek(), top, points[i]) <= 0) {
                top = hull.pop();
            }
            hull.push(top);
            hull.push(points[i]);
        }
    }

    /**
     * @return An iterable collection of points representing the convex hull.
     */
    public Iterable<Point> hull() {
        return new ArrayList<>(hull);
    }

    public record Point(int x, int y) implements Comparable<Point> {

        /**
         * Default constructor
         * @param x x-coordinate
         * @param y y-coordinate
         */
        public Point {
        }

        /**
         * @return the x-coordinate
         */
        @Override
        public int x() {
            return x;
        }
Repository: TheAlgorithms/Java
File: src/main/java/com/thealgorithms/conversions/AffineConverter.java (Java)

Code before:
package com.thealgorithms.conversions;

public final class AffineConverter {
    final double slope;
    final double intercept;
    public AffineConverter(final double inSlope, final double inIntercept) {
        slope = inSlope;
        intercept = inIntercept;
    }

    public double convert(final double inValue) {
        return slope * inValue + intercept;
    }

    public AffineConverter invert() {
        assert slope != 0.0;
        return new AffineConverter(1.0 / slope, -intercept / slope);
    }

    public AffineConverter compose(final AffineConverter other) {
        return new AffineConverter(slope * other.slope, slope * other.intercept + intercept);
    }
}


Review comment (suggestion):
Maybe it would be more clear and structured to have its own package for this (eg units) inside the conversions? And also number systems (eg numsystems)? Because these classes pretty special "class" of convertors in many other converters. And in the future, if someone wants to add something, it will be easier to find.

Code after:
package com.thealgorithms.conversions;

public final class AffineConverter {
    private final double slope;
    private final double intercept;
    public AffineConverter(final double inSlope, final double inIntercept) {
        slope = inSlope;
        intercept = inIntercept;
    }

    public double convert(final double inValue) {
        return slope * inValue + intercept;
    }

    public AffineConverter invert() {
        assert slope != 0.0;
        return new AffineConverter(1.0 / slope, -intercept / slope);
    }

    public AffineConverter compose(final AffineConverter other) {
        return new AffineConverter(slope * other.slope, slope * other.intercept + intercept);
    }
}
Repository: TheAlgorithms/Java
File: src/main/java/com/thealgorithms/conversions/BinaryToOctal.java (Java)

Code before:
package com.thealgorithms.conversions;

/**
 * Converts any Binary number to an Octal Number
 *
 * @author Zachary Jones
 */
public final class BinaryToOctal {
    private static final int BITS_IN_GROUP = 3;
    private static final int BINARY_BASE = 2;
    private static final int DECIMAL_BASE = 10;

    private BinaryToOctal() {
    }

    /**
     * This method converts a binary number to an octal number.
     *
     * @param binary The binary number
     * @return The octal number
     */
    public static String convertBinaryToOctal(int binary) {
        if (binary == 0) {
            return "0";
        }

        StringBuilder octal = new StringBuilder();
        int currentBit;
        int bitValueMultiplier = 1;

        while (binary != 0) {
            int octalDigit = 0;
            for (int i = 0; i < BITS_IN_GROUP && binary != 0; i++) {
                currentBit = binary % DECIMAL_BASE;

Review comment (suggestion):
The constant `BITS_IN_GROUP ` could be renamed to `BITS_PER_OCTAL_DIGIT` to make its purpose more explicit and improve readability.

Code after:
package com.thealgorithms.conversions;

public final class BinaryToOctal {
    private static final int BITS_PER_OCTAL_DIGIT = 3;
    private static final int BINARY_BASE = 2;
    private static final int DECIMAL_BASE = 10;

    private BinaryToOctal() {
    }

    /**
     * This method converts a binary number to an octal number.
     *
     * @param binary The binary number
     * @return The octal number
     * @throws IllegalArgumentException if the input is not a valid binary number
     */
    public static String convertBinaryToOctal(int binary) {
        if (binary == 0) {
            return "0";
        }

        if (!String.valueOf(binary).matches("[01]+")) {
            throw new IllegalArgumentException("Input is not a valid binary number.");
        }

        StringBuilder octal = new StringBuilder();
        int currentBit;
        int bitValueMultiplier = 1;

        while (binary != 0) {
            int octalDigit = 0;
            for (int i = 0; i < BITS_PER_OCTAL_DIGIT && binary != 0; i++) {
                currentBit = binary % DECIMAL_BASE;
Repository: TheAlgorithms/Java
File: src/main/java/com/thealgorithms/strings/Manacher.java (Java)

Code before:
package com.thealgorithms.strings;

/**
 * Wikipedia: https://en.wikipedia.org/wiki/Longest_palindromic_substring#Manacher's_algorithm
 */
public final class Manacher {

    // Private constructor to prevent instantiation
    private Manacher() {
    }

    /**
     * Test code for Manacher's Algorithm
     */
    public static void main(String[] args) {
        assert longestPalindrome("babad").equals("bab") || longestPalindrome("babad").equals("aba");
        assert longestPalindrome("cbbd").equals("bb");
        assert longestPalindrome("a").equals("a");
        assert longestPalindrome("ac").equals("a") || longestPalindrome("ac").equals("c");
    }

    /**
     * Finds the longest palindromic substring using Manacher's Algorithm
     *
     * @param s The input string
     * @return The longest palindromic substring in {@code s}
     */
    public static String longestPalindrome(String s) {
        // Preprocess the string to avoid even-length palindrome issues
        String processedString = preprocess(s);
        int n = processedString.length();
        int[] p = new int[n]; // Array to store the radius of palindromes

        // Separate variable declarations into individual statements
        int center = 0;
        int rightBoundary = 0;
        int maxLen = 0;
        int centerIndex = 0;

        // Iterate over the preprocessed string to calculate the palindrome radii

Review comment (suggestion):
Since you already have tests for this class, the main method seems unnecessary. The tests should cover the functionality you're aiming to verify. If someone wants to test something manually, they can easily add a main method themselves. However, given that this is an algorithm library, including a main method here feels redundant.

Code after:
package com.thealgorithms.strings;

/**
 * Wikipedia: https://en.wikipedia.org/wiki/Longest_palindromic_substring#Manacher's_algorithm
 */
public final class Manacher {

    private Manacher() {
    }

    /**
     * Finds the longest palindromic substring using Manacher's Algorithm
     *
     * @param s The input string
     * @return The longest palindromic substring in {@code s}
     */
    public static String longestPalindrome(String s) {
        final String processedString = preprocess(s);
        int[] palindromeLengths = new int[processedString.length()];
        int center = 0;
        int rightBoundary = 0;
        int maxLen = 0;
        int centerIndex = 0;

        for (int i = 1; i < processedString.length() - 1; i++) {
            int mirror = 2 * center - i;

            if (i < rightBoundary) {
                palindromeLengths[i] = Math.min(rightBoundary - i, palindromeLengths[mirror]);
            }

            while (processedString.charAt(i + 1 + palindromeLengths[i]) == processedString.charAt(i - 1 - palindromeLengths[i])) {
                palindromeLengths[i]++;
            }

            if (i + palindromeLengths[i] > rightBoundary) {
                center = i;
                rightBoundary = i + palindromeLengths[i];
            }
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 the array is empty or has only one element, return its length.
        if (nums == null || nums.length == 0) {
            return 0;
        }

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

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

        // Initialize dp array
        for (int i = 0; i < n; i++) {
            dp[i] = new HashMap<>();
            for (int j = 0; j < i; j++) {
                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
        final int n = nums.length;
```
I am not sure if creating this variable is needed - there is not overhead of using `nums.length` directly.

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:
     *
     * 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 the array is empty or has only one element, return its length.
        if (nums == null || nums.length == 0) {
            return 0;
        }

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

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

        // Initialize dp array
        for (int i = 0; i < n; i++) {
            dp[i] = new HashMap<>();
            for (int j = 0; j < i; j++) {
                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
                final int diff = nums[i] - nums[j];
```

Code after:
     * 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/dynamicprogramming/LongestArithmeticSubsequenceTest.java (Java)

Code before:
package com.thealgorithms.dynamicprogramming;

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

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 LongestArithmeticSubsequenceTest {

    @ParameterizedTest
    @MethodSource("provideTestCases")
    void testGetLongestArithmeticSubsequenceLength(int[] nums, int expected) {
        assertEquals(expected, LongestArithmeticSubsequence.getLongestArithmeticSubsequenceLength(nums));
    }

    private static Stream<Arguments> provideTestCases() {
        return Stream.of(Arguments.of(new int[] {3, 6, 9, 12, 15}, 5), Arguments.of(new int[] {1, 7, 10, 13, 14, 19}, 4), Arguments.of(new int[] {1, 2, 3, 4}, 4), Arguments.of(new int[] {}, 0), Arguments.of(new int[] {10}, 1), Arguments.of(new int[] {9, 4, 7, 2, 10}, 3));
    }
}


Review comment (suggestion):
```suggestion

        @ParameterizedTest
    @MethodSource("provideTestCases")
    void testGetLongestArithmeticSubsequenceLengthReversedInput(int[] nums, int expected) {
        ArrayUtils.reverse(nums);
        assertEquals(expected, LongestArithmeticSubsequence.getLongestArithmeticSubsequenceLength(nums));
    }

```

Make sure to:
```java
import org.apache.commons.lang3.ArrayUtils;
```

Code after:

import java.util.stream.Stream;
import org.apache.commons.lang3.ArrayUtils;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

public class LongestArithmeticSubsequenceTest {
    @ParameterizedTest
    @MethodSource("provideTestCases")
    void testGetLongestArithmeticSubsequenceLength(int[] nums, int expected) {
        assertEquals(expected, LongestArithmeticSubsequence.getLongestArithmeticSubsequenceLength(nums));
    }
    @ParameterizedTest
    @MethodSource("provideTestCases")
    void testGetLongestArithmeticSubsequenceLengthReversedInput(int[] nums, int expected) {
        ArrayUtils.reverse(nums);
        assertEquals(expected, LongestArithmeticSubsequence.getLongestArithmeticSubsequenceLength(nums));
    }

    @Test
    void testGetLongestArithmeticSubsequenceLengthThrowsForNullInput() {
        assertThrows(IllegalArgumentException.class, () -> LongestArithmeticSubsequence.getLongestArithmeticSubsequenceLength(null));
    }

    private static Stream<Arguments> provideTestCases() {
        return Stream.of(Arguments.of(new int[] {3, 6, 9, 12, 15}, 5), Arguments.of(new int[] {1, 7, 10, 13, 14, 19}, 4), Arguments.of(new int[] {1, 2, 3, 4}, 4), Arguments.of(new int[] {}, 0), Arguments.of(new int[] {10}, 1), Arguments.of(new int[] {9, 4, 7, 2, 10}, 3),
            Arguments.of(new int[] {1, 2, 2, 2, 2, 5}, 4));
    }
}