Freecodecamp: Wrong solution is accepted when submitted

[guyziv2110]

Challenge Return Largest Numbers in Arrays has an issue.
User Agent is: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/55.0.2883.87 Safari/537.36.
The code below uses map/reduce as suggested in the medium forum (I've reviewed the solutions over there) and is accepted when submitted.

The problem is that there is an assumption that in every subarray not all of the values are negatives and it is not shown in the description of the problem. That is, if one of the subarray contains only negatives values, the accepted solution below isn't correct.

My code:

function largestOfFour(arr) {
  return arr.map(function(subArray) {
    return subArray.reduce(function(prev, curr) {
      return (curr > prev ? curr : prev);
    }, 0);
  });
}

largestOfFour([[4, 5, 1, 3], [13, 27, 18, 26], [32, 35, 37, 39], [1000, 1001, 857, 1]]);

[ghost]

@guyziv2110
So if I get you right, your problem is that when 1 array is full of only negative numbers the output is not correct.

I tried that and as for others to see, the output should be -13 as a highest number, but the output shows 0 is the highest one.

[Bouncey]

@guyziv2110 Thank you for reporting this. Indeed the second argument for reduce should be Number.NEGATIVE_INFINITY or some other extremely low number to counter this case.

Are the solution examples you found owned by us? I mean, are they on our forum/wiki?

cc @FreeCodeCamp/moderators Is this worthy of a new test case? I would say yes, but we would possibly need to update to wiki also.

[guyziv2110]

@Bouncey Yes. After I solved the problem by myself I walked through the solution for this problem posted in medium (https://medium.freecodecamp.com/three-ways-to-return-largest-numbers-in-arrays-in-javascript-5d977baa80a1) and found this issue.

[Bouncey]

Although that article is on the FreeCodeCamp medium blog, I would think it is owned by @SonyaMoisset.

@QuincyLarson would it be Sonya that needs to make the edit if she agreed to it?

[ooade]

@Bouncey We should add a test case that includes negative numbers in a sub array to make that solution fail :sweat_smile:

[Bouncey]

First-timers!!

Add a new test with an array containing only negative numbers i.e -24

The code for this challenge is here

Be sure to follow CONTRIBUTING.md

And hit us up in Contributor Chat if you have any problems!

Good luck and Happy Coding!

[ooade]

@Bouncey :+1:

[rajpatel507]

@Bouncey can I take this one?

[Bouncey]

:+1:

Happy Coding!

On Monday, 9 January 2017, Rajesh Kathiriya [e-mail]
wrote:

@Bouncey https://github.com/Bouncey can I take this one?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/FreeCodeCamp/FreeCodeCamp/issues/12419#issuecomment-271270310,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ARtk5p7Dw0TOnXWqGkGyfyUQSz8PBSAJks5rQiSngaJpZM4LdbH0
.

[91aman]

@Bouncey I have added a test case for negative values in the array. Please do review the above PR.

[dhcodes]

@91aman in the future, please comment on the thread before working on the PR. @rajpatel507 looks like there is a PR in process at this point. I'm sure we'll have more first-timer issues soon, or feel welcome to look at our issue backlog.

[91aman]

@dhcodes Duly noted. will Inform in the thread before working on any issues.

[systimotic]

I'd like that to fix this in the code you don't need Number.NEGATIVE_INFINITY, removing the , 0 is enough.
prev will then be undefined on the first run. Nothing is larger than undefined, therefore it will always return false and use curr on the firsts run.

[91aman]

@systimotic I agree with you, not giving initial value 0 will work. The explanation is not correct.

If 0 is not given, prev would not be undefined, In the first run, prev would be the first value of the array, and the curr would be the second value of the array. The comparison would be between the first 2 values of the array.

[systimotic]

@91aman That was a stupid mistake, sorry. 😳 You're absolutely right, thanks for reminding me! ❤️

[dhcodes]

Just to be clear before I merge this in, making this change will nullify the basic and intermediate solutions on the wiki page for this challenge; only the advanced solution passes the added "all negatives" test. https://forum.freecodecamp.com/t/algorithm-return-largest-numbers-in-arrays/16042

Seems if we make this change, we should also change the solutions on the wiki article.

[91aman]

@dhcodes I agree, we will need to change both the basic and intermediate solutions. Can you give me some pointers as to where we have the source code of the wiki article? I can fix it.

[raisedadead]

@91aman it's in his comments above yours:

https://forum.freecodecamp.com/t/algorithm-return-largest-numbers-in-arrays/16042

[systimotic]

@91aman I made the edit on the forum 🙂

[91aman]

@systimotic Thanks, I see that you have changed the intermediate solutions. We will also need to change the basic solution where we are still doing var largestNumber = 0;.

we can do var largestNumber = arr[n][0]; and then start sb from 1.

function largestOfFour(arr) {
  var results = [];
  for (var n = 0; n < arr.length; n++) {
    var largestNumber = arr[n][0];
    for (var sb = 1; sb < arr[n].length; sb++) {
      if (arr[n][sb] > largestNumber) {
        largestNumber = arr[n][sb];
      }
    }

    results[n] = largestNumber;
  }

  return results;
}

[systimotic]

Wow, I'm really slacking on this issue! Thanks @91aman, I updated and tested that as well.

[91aman]

@systimotic Awesome, Thanks :)