-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
add maximum.rs #725
add maximum.rs #725
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #725 +/- ##
==========================================
- Coverage 94.96% 94.95% -0.01%
==========================================
Files 303 304 +1
Lines 22563 22559 -4
==========================================
- Hits 21426 21422 -4
Misses 1137 1137 ☔ View full report in Codecov by Sentry. |
src/big_integer/maximum.rs
Outdated
pub fn maximum(mut nums: Vec<i32>) -> i32 { | ||
nums.sort(); | ||
(nums[nums.len() - 1] * nums[nums.len() - 2] * nums[nums.len() - 3]) | ||
.max(nums[0] * nums[nums.len() - 1] * nums[nums.len() - 2]) | ||
.max(nums[0] * nums[1] * nums[nums.len() - 1]) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The complexity of this is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file definitely does not belong to the big_integer
directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
math? or mkdir array?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Under src create a array under src would be a good choice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your PR is solving a problem from Leetcode, which often leads to unknowing algorithm categories. So we need to know the target of the algorithm (or data structure) so that we can clarify it into the best-suited category. In case, there are no suitable categories, we need to discuss whether it is necessary to create a new directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- What do you think about change the type of elements from
i32
toisize
(you may need to handle larger ranges of integer values or if your algorithm benefits from being able to handle platform-dependent integer sizes, which vary between 32-bit and 64-bit platforms) - What do you think about renaming and adding some more tests, here is my suggestions (feel free to modify or apply on your own):
test_maximum! { three_positive_numbers: ([1, 2, 3], 6), four_positive_numbers: ([1, 2, 3, 4], 24), four_numbers_with_zero: ([-1, 2, 0, 4], 0), three_negative_numbers: ([-1, -2, -3], -6), four_negative_numbers: ([-1, -2, -3, -4], -6), five_negative_numbers: ([-1, -2, -3, -4, -5], -6), negative_numbers_with_zero: ([-1, -2, 0, -3, -4, -5], 0), four_negative_numbers_with_zero: ([-1, -2, 0, -3, -4], 0), mixed_positive_and_negative_numbers1: ([-1, -2, -3, 4, 5], 30), mixed_positive_and_negative_numbers2: ([-1, 2, -3, -4, 5], 60), mixed_positive_and_negative_numbers3: ([-1, 2, -3, 4, 5], 40), single_negative_number_repeated: ([-1, -1, -1, -1, -1], -1), all_zeros: ([0, 0, 0, 0, 0], 0), two_positive_one_negative: ([1, 2, -3], -6), two_negative_one_positive: ([-1, -2, 3], 6), large_numbers: ([1000, 1000, 10000], 10000000000), mixture_with_large_and_small_numbers: ([1, -1, 2, -2, 1000000], 2000000), }
Calculate the maximum set of multiplications of three numbers in an array