Commit de729294 authored by Yutaka Hirano's avatar Yutaka Hirano Committed by Commit Bot

Update blink C++ style guide in regard to bool params

Discussion: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/GF1Wg0JbEhw

Bug: 983200
Change-Id: I27e02f2b195a93a7f2b0630af632ec0122581da9
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1697481Reviewed-by: default avatarJeremy Roman <jbroman@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#678265}
parent 6a9fb0db
...@@ -191,16 +191,58 @@ class Node { ...@@ -191,16 +191,58 @@ class Node {
}; };
``` ```
## Prefer enums to bools for function parameters ## Prefer enums or StrongAliases to bare bools for function parameters
Prefer enums to bools for function parameters if callers are likely to be Prefer enums to bools for function parameters if callers are likely to be
passing constants, since named constants are easier to read at the call site. passing constants, since named constants are easier to read at the call site.
An exception to this rule is a setter function, where the name of the function Alternatively, you can use base::util::StrongAlias<Tag, bool>. An exception to
already makes clear what the boolean is. this rule is a setter function, where the name of the function already makes
clear what the boolean is.
**Good:** **Good:**
```c++ ```c++
class FrameLoader {
public:
enum class CloseType {
kNotForReload,
kForReload,
};
bool ShouldClose(CloseType) {
if (type == CloseType::kForReload) {
...
} else {
DCHECK_EQ(type, CloseType::kNotForReload);
...
}
}
};
// An named enum value makes it clear what the parameter is for. // An named enum value makes it clear what the parameter is for.
if (frame_->Loader().ShouldClose(CloseType::kNotForReload)) { if (frame_->Loader().ShouldClose(FrameLoader::CloseType::kNotForReload)) {
// No need to use enums for boolean setters, since the meaning is clear.
frame_->SetIsClosing(true);
// ...
```
**Good:**
```c++
class FrameLoader {
public:
using ForReload = base::util::StrongAlias<class ForReloadTag, bool>;
bool ShouldClose(ForReload) {
// A StrongAlias<_, bool> can be tested like a bool.
if (for_reload) {
...
} else {
...
}
}
};
// Using a StrongAlias makes it clear what the parameter is for.
if (frame_->Loader().ShouldClose(FrameLoader::ForReload(false))) {
// No need to use enums for boolean setters, since the meaning is clear. // No need to use enums for boolean setters, since the meaning is clear.
frame_->SetIsClosing(true); frame_->SetIsClosing(true);
...@@ -209,6 +251,17 @@ if (frame_->Loader().ShouldClose(CloseType::kNotForReload)) { ...@@ -209,6 +251,17 @@ if (frame_->Loader().ShouldClose(CloseType::kNotForReload)) {
**Bad:** **Bad:**
```c++ ```c++
class FrameLoader {
public:
bool ShouldClose(bool for_reload) {
if (for_reload) {
...
} else {
...
}
}
};
// Not obvious what false means here. // Not obvious what false means here.
if (frame_->Loader().ShouldClose(false)) { if (frame_->Loader().ShouldClose(false)) {
frame_->SetIsClosing(ClosingState::kTrue); frame_->SetIsClosing(ClosingState::kTrue);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment