Skip to content

[BUG]: MaxProductOfThree not working properly #1294

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

Closed
Xolvez opened this issue Feb 18, 2023 · 1 comment · Fixed by #1295
Closed

[BUG]: MaxProductOfThree not working properly #1294

Xolvez opened this issue Feb 18, 2023 · 1 comment · Fixed by #1295
Labels
bug Something isn't working

Comments

@Xolvez
Copy link
Contributor

Xolvez commented Feb 18, 2023

Description

The maxProductOfThree-function in Dynamic-Programming/MaxProductOfThree.js is not working properly when the values of the array vary around 0.

I noticed something seemed off when I looked at the code for the function, and decided to test the function using random arrays of length 4, where the values varied between -3 and 3. The output was compared to the output of a function that calculated all possible products of 3 numbers in the array, and indeed the function produced incorrect outputs.

Examples arrays that produce incorrect output:
[-2,1,-1,1]: expected = 2, actual = -1.
[-3,0,-1,1]: expected = 3, actual = 0.
[-1,1,-1,1]: expected = 1, actual = -1.

Expected Behavior

The expected behavior of the function is to find the largest product that can be produced from 3 numbers of an array. Due to the implementation, it is currently not.

Expected output of the following arrays of length 4:
[-2,1,-1,1]: expected = 2.
[-3,0,-1,1]: expected = 3.
[-1,1,-1,1]: expected = 1.

Actual Behavior

Actual output of the arrays above:
[-2,1,-1,1]: actual = -1.
[-3,0,-1,1]: actual = 0.
[-1,1,-1,1]: actual = -1.

Steps to reproduce (if applicable)

  1. Call the function maxProductOfThree with the input [-2, 1, -1, 1].
  2. Observe that the expected output is clearly 2, since (-2) * (-1) * 1 = 2
  3. Observe that the actual output is -1.

Additional information

I suspect this has to do with the way the max and min variables are initialized to -1 in the function, instead of setting them to null to begin with.

@Xolvez Xolvez added the bug Something isn't working label Feb 18, 2023
@Xolvez
Copy link
Contributor Author

Xolvez commented Feb 18, 2023

I am currently working on a fix for this, just formating the code to comply with standardjs at the moment.

Xolvez added a commit to Xolvez/JavaScript that referenced this issue Feb 18, 2023
Fixed the error in the MaxProductOfThree by initializing the max and min
variables to null instead of -1. The checks were then altered to check
for null instead of -1.

Also wrote more tests, which randomly generated small arrays and
compared the output of the maxProductOfThree-algorithm to the output of
a slower, but complete, function which calculates all posible
triple-products of the values of the array.

Fixes: TheAlgorithms#1294
raklaptudirm pushed a commit that referenced this issue Feb 18, 2023
* fix: fixed error in the MaxProductOfThree algorithm

Fixed the error in the MaxProductOfThree by initializing the max and min
variables to null instead of -1. The checks were then altered to check
for null instead of -1.

Also wrote more tests, which randomly generated small arrays and
compared the output of the maxProductOfThree-algorithm to the output of
a slower, but complete, function which calculates all posible
triple-products of the values of the array.

Fixes: #1294

* fix: Added newlines at the end of the files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant