Bug 63564 – coding-style: Add note about cases where enums are preferred over bools

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Leandro Pereira]

coding-style: Add note about cases where enums are preferred over bools

[Leandro Pereira]

Created attachment 98979 [details]
Patch

[Darin Adler]

Comment on attachment 98979 [details]
Patch

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

> Websites/webkit.org/coding/coding-style.html:606
> +<li>On functions with more than one parameter, prefer enums to bools if callers are likely to be passing constants.

This rule applies to functions with one parameter too. The number of parameters is not the key.

Maybe the reason you though it depended on the number of parameters is that it does not apply to a setXXX function where the name of the function makes clear what the boolean is.

I also think the guideline should explain the reason. The code below does allude to the reason this matters, but it’s worth saying that the named constants are much easier to read at the call site.

[Darin Adler]

Comment on attachment 98979 [details]
Patch

review- because I’d like to remove the mistaken “more than one parameter”

[Leandro Pereira]

Created attachment 99083 [details]
Patch

[Darin Adler]

Comment on attachment 99083 [details]
Patch

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

Thanks. Getting better.

> Websites/webkit.org/coding/coding-style.html:607
> +passing constants, since named constants are easier to read on the call site. 

Should be "at the call site" rather than "on the call site".

> Websites/webkit.org/coding/coding-style.html:608
> +The only exception would be on setters, where the name of the function

Instead of "The only exception would be on setters," I would say, "An exception to this rule is a setter function,"

> Websites/webkit.org/coding/coding-style.html:613
> +doSomething(something, AllowFooBar);
> +paintTextWithShadows(context, ..., textStrokeWidth > 0, isHorizontal());

The right example should include a setter that takes a boolean.

> Websites/webkit.org/coding/coding-style.html:618
> +doSomething(something, false);

The wrong example should include a setter that takes an enum.

[Leandro Pereira]

Created attachment 99107 [details]
Patch

[Leandro Pereira]

(In reply to comment #5)
> 
> Should be "at the call site" rather than "on the call site".
> Instead of "The only exception would be on setters," I would say, "An exception to this rule is a setter function,"
> The right example should include a setter that takes a boolean.
> The wrong example should include a setter that takes an enum.
>

Fixed everything. Thanks.

[WebKit Review Bot]

The commit-queue encountered the following flaky tests while processing attachment 99107 [details]:

http/tests/media/reload-after-dialog.html bug 59901 (authors: [e-mail], [e-mail], [e-mail], and [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Review Bot]

Comment on attachment 99107 [details]
Patch

Clearing flags on attachment: 99107

Committed r90027: <http://trac.webkit.org/changeset/90027>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.