6
\$\begingroup\$

This works, and I personally find it elegant and kind of "neat," but then I worry it's a bit too tricky. Comments welcome:

function (reports) {
    reports.forEach(function (report, i) {
        report.upPercentage = report.upTime / report.totalTime;
        report.rank = i; // temporary storage in a convenient location
    });
    _.sortBy(reports, 'upPercentage').forEach(function (report, i) {
        reports[report.rank].rank = i; // the 'trick'
    });
    return reports;
}

The first loop goes through the array of reports, and calculates the upPercentage for each report. This part should arguably be done on the backend but it isn't and it isn't going to be. Not my call.

The first loop also assigns each element's own position in the array to its rank. This is not the report's actual rank, it's just data I need for the second loop.

The second loop is iterating over a sorted copy of the reports array (it is important that the original sorting of reports be maintained), and uses the element's original position, stored in rank, to find it in the original, unsorted array. It then assigns the element's position in the sorted array to the rank of the element in the original, unsorted array.

Thus rank ends up being equal to the number of reports that have a lower upPercentage than does the given report.

\$\endgroup\$
  • \$\begingroup\$ I'm guessing this is just a typo, but wouldn't report.upPercentage = reports.upTime / reports.totalTime result in every report having the same upPercentage? Is upTime supposed to be a property of the reports collection or of an individual report? \$\endgroup\$ – Thriggle Feb 28 '15 at 17:23
  • \$\begingroup\$ @Thriggle Ah, yeah, typo in the creation of my SSCCE. \$\endgroup\$ – KRyan Feb 28 '15 at 19:33
6
\$\begingroup\$

Forgive me if I misunderstand it, but in the second loop isn't the report and reports[report.rank] refer the same object?

Because if so, then the following simpler code would produce the same result:

function (reports) {
    reports.forEach(function (report, i) {
        report.upPercentage = reports.upTime / reports.totalTime;
    });
    _.sortBy(reports, 'upPercentage').forEach(function (report, i) {
        report.rank = i;
    });
    return reports;
}
\$\endgroup\$
  • 1
    \$\begingroup\$ ...this raises the excellent question of whether or not _.sortBy performs a deep copy or not. I had been assuming it did, but that's not a fair assumption. I will test it. \$\endgroup\$ – KRyan Feb 27 '15 at 20:52
  • \$\begingroup\$ Hmmm ... I was assuming the contrary. Yet the reference is not stating this explicitly. underscorejs.org/#sortBy \$\endgroup\$ – Gábor Angyal Feb 27 '15 at 20:54
  • \$\begingroup\$ It does not, which is honestly more consistent with JavaScript in general. Your code works. Thank you very much. Clearly superior. \$\endgroup\$ – KRyan Feb 27 '15 at 20:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.