I'd like to propose a change to the WebKit Code Style Guidelines (https://webkit.org/code-style-guidelines/) specifically around this rule: https://webkit.org/code-style-guidelines/#linebreaking-else-if. The current rules state that "An else if statement should be written as an if statement when the prior if concludes with a return statement." Additionally, this is generally construed to apply to else statements after a return being elided entirely as well. e.g. if (condition) return foo; else return bar; should be written as: if (condition) return foo; return bar; I think we should remove this rule and leave it up to the writer to choose how to style this, allowing both else-if after a return and else after a return. My arguments for allowing this are: - It is often easier to follow logic if you can read out the "if, else-if, else-if, else", rather than having to notice that there is a return in the statement and adding the else in your head. - For single line conditions like the above example, the lining up of the return statements is pleasing. - For constexpr if, the elses are required, so we have to break the rule for them anyway. I'm eager to paint this bike shed, so what do you think?
I did run into this recently when introducing an "if constexpr" and had to ignore the rule then. That said, unless forced to by constexpr, I actually like that not using an else statement reduces the nesting. It is the same reason we like early returns in WebKit. I think this would look terrible for e.g.: ``` if (myEarlyReturnCondition) return; else { // My // long // function // body. } ``` For single line conditions, the having the else statement doesn't look bad but I don't find it more pleasing to have the else statement :) I also personally don't find it confusing or harder to follow-up that we don't have an else after an early return. Anyway, if others disagree with me fine but my 2 cents is that I prefer the current coding style (except in the very specific case of if constexpr where we don't have any other choice).
I like early return when it’s a true "early exit". I think it can make it much easier to follow the logic of a function. I don’t think we should insist on it when the two sides of the condition are parallel, especially when both are simple one-liners. In practice, we are not enforcing an early return style.
(In reply to Chris Dumez from comment #1) > I did run into this recently when introducing an "if constexpr" and had to > ignore the rule then. That said, unless forced to by constexpr, I actually > like that not using an else statement reduces the nesting. It is the same > reason we like early returns in WebKit. > > I think this would look terrible for e.g.: > ``` > if (myEarlyReturnCondition) > return; > else { > // My > // long > // function > // body. > } > ``` > I agree this would be bad, and I don't think we should have a rule to require this. I am very much proposing this. I am just proposing that we call this decision a "styleistic choice". I happen to think something like: ``` if (aCondition) { // Do // Something // here return foo; } else if (anotherCondition) { // Do // Something // else return bar; } else { // Do // a. // third thing return baz; } ``` is quite pleasing. Especially since if you didn't have the early returns, we would write it: if (aCondition) { // Do // Something // here } else if (anotherCondition) { // Do // Something // else } else { // Do // a. // third thing } > For single line conditions, the having the else statement doesn't look bad > but I don't find it more pleasing to have the else statement :) > > I also personally don't find it confusing or harder to follow-up that we > don't have an else after an early return. I am not really arguing that it is confusing, just that in some cases it is more pleasing to have the else. switch (format.pixelFormat) { case PixelFormat::RGBA8: if (format.alphaFormat == AlphaPremultiplication::Premultiplied) return std::make_tuple(8u, 32u, kCGBitmapByteOrder32Big | kCGImageAlphaPremultipliedLast); else return std::make_tuple(8u, 32u, kCGBitmapByteOrder32Big | kCGImageAlphaLast); case PixelFormat::BGRA8: if (format.alphaFormat == AlphaPremultiplication::Premultiplied) return std::make_tuple(8u, 32u, kCGBitmapByteOrder32Little | kCGImageAlphaPremultipliedFirst); else return std::make_tuple(8u, 32u, kCGBitmapByteOrder32Little | kCGImageAlphaFirst); case PixelFormat::RGB10: case PixelFormat::RGB10A8: break; } looks more pleasing to me and a bit easier to follow than switch (format.pixelFormat) { case PixelFormat::RGBA8: if (format.alphaFormat == AlphaPremultiplication::Premultiplied) return std::make_tuple(8u, 32u, kCGBitmapByteOrder32Big | kCGImageAlphaPremultipliedLast); return std::make_tuple(8u, 32u, kCGBitmapByteOrder32Big | kCGImageAlphaLast); case PixelFormat::BGRA8: if (format.alphaFormat == AlphaPremultiplication::Premultiplied) return std::make_tuple(8u, 32u, kCGBitmapByteOrder32Little | kCGImageAlphaPremultipliedFirst); return std::make_tuple(8u, 32u, kCGBitmapByteOrder32Little | kCGImageAlphaFirst); case PixelFormat::RGB10: case PixelFormat::RGB10A8: break; } (this is really a minor thing, it's just something I commonly get wrong due to my desire for parallel construction).
Yeah, allowing else clause in the case multiple conditions are parallel to one another makes sense to me. But rather than leaving it to author & reviewed to make arbitrary decision, can we codify that? I'm afraid some people will ignore this and start arguing / writing code without an early return even in the case where an early return and no-else clause is more appropriate because there is no logical parallelism between the two.
<rdar://problem/78318197>