[Webkit-unassigned] [Bug 182387] StyleBench: Attribute selectors and other improvements

[bugzilla-daemon at webkit.org]

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

Joseph Pecoraro <joepeck at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #333279|review?                     |review+
              Flags|                            |

--- Comment #3 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 333279
  --> https://bugs.webkit.org/attachment.cgi?id=333279
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=333279&action=review

r=me

> PerformanceTests/StyleBench/resources/style-bench.js:203
> +        if (valueNum==0)
> +            return "";
> +        if (valueNum==1)
> +            return "val";

Normal style would be:

    if (valueNum === 0)
        return "";
    if (valueNum === 1)
        return "val";

Up to you if you want to converge on `==` or `===`.

> PerformanceTests/StyleBench/resources/style-bench.js:204
> +        return `val${ valueNum }`;

Style: Normally you don't use spaces inside the ${...}.

> PerformanceTests/StyleBench/resources/style-bench.js:233
> +        if (operator=='')

Ditto on style. Or here you could even just:

    if (!operator)

> PerformanceTests/StyleBench/resources/style-bench.js:266
> +            result +=  this.randomAttributeSelector();

Style: Accidental double space after the operator.

> PerformanceTests/StyleBench/resources/style-bench.js:493
> +                const count = this.random.number(this.configuration.elementMaximumAttributes) + 1;
> +                for (let i = 0; i < count; ++i)
> +                    element.setAttribute(this.randomAttributeName(), this.randomAttributeValue());

It is bad style to shadow both `i` and `count`. Had you used `var` you'd have run into issues! I'd recommend using different names just for clarity.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180207/1ef9a918/attachment.html>