[Webkit-unassigned] [Bug 147667] Refactor BuildbotQueue.compareIterations and BuildbotQueue.compareIterationsByRevisions to be more generic

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Aug 7 13:51:16 PDT 2015


https://bugs.webkit.org/show_bug.cgi?id=147667

--- Comment #4 from Jason Marcell <jmarcell at apple.com> ---
(In reply to comment #2)
> > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/Dashboard.js:68
> > +    _sortedItems: function(unsorted)
> > +    {
> > +        var sorted = [];
> > +
> > +        for (var key in unsorted)
> > +            sorted.push(unsorted[key]);
> > +
> > +        sorted.sort(function(a, b) {
> > +            return a.order - b.order;
> > +        });
> > +
> > +        return sorted;
> 
> Please make this a free function. Additionally, the name of this function is
> disingenuous given the presence of its argument. Specifically the use of the
> past tense word "sorted" implies the function is a getter and/or sorts some
> kind of global data "items". But it takes an unsorted array as its argument
> and sorts this argument. Functions that perform a computation should have a
> name that begins with a verb. Maybe sortDictionaryByOrder()? Can we come up
> with a better name?

Okay, but it's not really sorting a dictionary. It's really sorting an array of dictionaries or objects that happen to have an `order` key or property. Maybe `sortArrayByOrder` or `sortDictionariesByOrder` or `sortObjectsByOrder`?

Also, there is a Utilities.js file. Do you think this should go there?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150807/85324fe4/attachment-0001.html>


More information about the webkit-unassigned mailing list