[Webkit-unassigned] [Bug 118609] Web Inspector: Replace binarySearch with lowerBound and upperBound functions

[bugzilla-daemon at webkit.org]

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


Timothy Hatcher <timothy at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #224837|review?                     |review-
               Flag|                            |




--- Comment #3 from Timothy Hatcher <timothy at apple.com>  2014-02-21 09:30:22 PST ---
(From update of attachment 224837)
View in context: https://bugs.webkit.org/attachment.cgi?id=224837&action=review

Thanks for fixing this! Can you upload a new patch with these tweaks?

> Source/WebInspectorUI/ChangeLog:5
> +        Web Inspector: Replace binarySearch with lowerBound and upperBound functions
> +        https://bugs.webkit.org/show_bug.cgi?id=118609
> +

Can you add the reasoning bellow the title? "This makes insertionIndexForObjectInListSortedByFunction work in O(log(n)) time instead of O(n)."

> Source/WebInspectorUI/ChangeLog:13
> +        (.):
> +        (.value):
> +        (object):

Our script gets confused by Object.defineProperty. Can you manually fix this?

> Source/WebInspectorUI/UserInterface/Utilities.js:939
> +
> +

Double new line.

> Source/WebInspectorUI/UserInterface/Utilities.js:946
> +    /**
> +     * Return index of the leftmost element that is equal or greater
> +     * than the specimen object. If there's no such element (i.e. all
> +     * elements are smaller than the specimen) returns array.length.
> +     * The function works for sorted array.

Lets just use // for these comments.

> Source/WebInspectorUI/UserInterface/Utilities.js:951
> +     *
> +     * @this {Array.<*>}
> +     * @param {*} object
> +     * @param {function(*,*):number=} comparator
> +     * @return {number}

We don't use these JSDoc comments. We intend to remove them from our existing code. Lets not add more.

> Source/WebInspectorUI/UserInterface/Utilities.js:979
> +    /**
> +     * Return index of the leftmost element that is greater
> +     * than the specimen object. If there's no such element (i.e. all
> +     * elements are smaller than the specimen) returns array.length.
> +     * The function works for sorted array.

Ditto.

> Source/WebInspectorUI/UserInterface/Utilities.js:984
> +     *
> +     * @this {Array.<*>}
> +     * @param {*} object
> +     * @param {function(*,*):number=} comparator
> +     * @return {number}

Ditto.

> Source/WebInspectorUI/UserInterface/Utilities.js:991
> +        function defaultComparator(a, b)
> +        {
> +            return a - b;
> +        }

Lets move this function out next to insertObjectIntoSortedArray, since it can be shared.

> Source/WebInspectorUI/UserInterface/Utilities.js:1013
> +    /**
> +     * @this {Array.<*>}
> +     * @param {*} value
> +     * @param {function(*,*):number} comparator
> +     * @return {number}
> +     */

Ditto.

> Source/WebInspectorUI/UserInterface/Utilities.js:1027
> +/**
> + * @param {*} object
> + * @param {Array.<*>} list
> + * @param {function(*,*):number=} comparator
> + * @param {boolean=} insertionIndexAfter
> + * @return {number}
> + */

Ditto.

> Source/WebInspectorUI/UserInterface/Utilities.js:1041
> +/**
> + * @param {*} object
> + * @param {Array.<*>} array
> + * @param {function(*,*):number=} comparator
> + */

Ditto.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.