Add MaximumElement implementation with documentation#7318
Add MaximumElement implementation with documentation#7318AbhiramSakha wants to merge 2 commits intoTheAlgorithms:masterfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7318 +/- ##
============================================
- Coverage 79.43% 79.41% -0.03%
+ Complexity 7062 7061 -1
============================================
Files 788 789 +1
Lines 23124 23129 +5
Branches 4545 4547 +2
============================================
- Hits 18368 18367 -1
- Misses 4022 4027 +5
- Partials 734 735 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Hey! Thanks for taking the time to put this together.
While reviewing this, I noticed that we actually already have established implementations handling this exact logic in the repository (such as FindMax.javaand AbsoluteMax.java in the maths package).
To keep the codebase lean and avoid unnecessary redundancy, it might be best to withdraw this specific PR. If you are looking to contribute, I'd highly recommend checking out the open issues to see where help is needed most right now!
Also left some general suggestions for your code for future reference.
| * @param arr input array | ||
| * @return maximum value | ||
| */ | ||
| public static int max(int[] arr) { |
There was a problem hiding this comment.
While it is always great to see new contributions, there are a few critical edge cases and efficiency improvements needed here for a robust utility class:
- The method does not validate against
nullinputs. Passing anullarray will throw an unhandledNullPointerExceptionwhen the enhanced for-loop attempts to execute. - If an empty array (
new int[0]) is passed, the loop is bypassed entirely and the method returnsInteger.MIN_VALUE. Since an empty set technically has no maximum element, returning an arbitrary extreme integer can cause silent downstream bugs. This should throw anIllegalArgumentExceptioninstead. - Initializing
maxtoInteger.MIN_VALUErelies on a magic constant. A cleaner, more efficient algorithmic standard is to validate the array length, setmax = arr[0], and iterate starting from the second element.
There was a problem hiding this comment.
While it is always great to see new contributions, there are a few critical edge cases and efficiency improvements needed here for a robust utility class:
- The method does not validate against
nullinputs. Passing anullarray will throw an unhandledNullPointerExceptionwhen the enhanced for-loop attempts to execute.- If an empty array (
new int[0]) is passed, the loop is bypassed entirely and the method returnsInteger.MIN_VALUE. Since an empty set technically has no maximum element, returning an arbitrary extreme integer can cause silent downstream bugs. This should throw anIllegalArgumentExceptioninstead.- Initializing
maxtoInteger.MIN_VALUErelies on a magic constant. A cleaner, more efficient algorithmic standard is to validate the array length, setmax = arr[0], and iterate starting from the second element.
Update your code like this:
public static int max(int[] arr) {
if (arr == null) {
throw new IllegalArgumentException("Input array cannot be null");
}
if (arr.length == 0) {
throw new IllegalArgumentException("Array must contain at least one element");
}
int max = arr[0];
for (int i = 1; i < arr.length; i++) {
if (arr[i] > max) {
max = arr[i];
}
}
return max;
}
📌 Description
Added a new implementation to find the maximum element in an array.
✅ Changes Made
findMaxmethod using iteration🧪 Testing
📋 Checklist