Skip to content

Conversation

@foxbenjaminfox
Copy link
Contributor

I was trying to figure out what tie-breaker is used for the mode aggday. I went looking in the code, and found this comment:

road/src/butil.js

Lines 622 to 624 in e5fa3c7

/** Mode (commonest) of values in list a. Breaks ties in favor of whatever
appears first in the lsit. (Mathematica-brain gave the median of the list of
commonest elements but literally no one cares about aggday=mode anyway.)

(Fair enough to say that that no one cares about the mode aggday; but I need to care if I'm writing code that I want to be compatible with the way Beeminder does it.)

The comment does not accurately describe what the javascript function does.

mode([1, 2, 2, 1]) // Returns 2, not 1

What the current implementation actually does is break ties in favor of the one whose final occurrence appears first in the list. 1 appears in [1, 2, 2, 1] before 2 does, but the final occurrence of 2 comes before the final occurrence of 1.

This can be fixed either by changing the implementation, or by changing the comment. I chose to change the comment, even though the behavior in question is somewhat awkward to describe properly. If you'd prefer I instead fix the behavior to match the comment, I could do that instead.

Sorry for nitpicking something that "literally no one cares about", as the comment alleges, but as I did in fact need to spend a few minutes puzzling about the behavior here, it's probably best if it were accurately documented. (Better to have no comment at all than one that wrongly describes how the code works!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant