objective-c-style-guide icon indicating copy to clipboard operation
objective-c-style-guide copied to clipboard

Add segment about simple if statements with return

Open KATT opened this issue 11 years ago • 19 comments

More readable to do...

.. this
- (void)twoSidedViewWillFlip:(US2TwoSidedView *)view
{
    if (!view.isFrontFacing) return;

    // [..]
}
.. rather than this
- (void)twoSidedViewWillFlip:(US2TwoSidedView *)view
{
    if (!view.isFrontFacing)
    {
        return;
    }

    // [..]
}

KATT avatar Jan 16 '14 11:01 KATT

:+1:

martinstolz avatar Jan 16 '14 11:01 martinstolz

:thumbsup:

alexfish avatar Jan 16 '14 11:01 alexfish

Such a ruby dev.

Don't we open a can of worms by allowing that?

iceydee avatar Jan 16 '14 11:01 iceydee

Don't we open a can of worms by allowing that?

Why?

KATT avatar Jan 16 '14 11:01 KATT

After that comes:

if (something)
  do this
else if
  do this
else
  do this third

And then we will have the inevitable (bug):

if (something)
  do this
else if
  do this
else
  do this
  do this, expect to only happen in else, but will actually always get executed.

iceydee avatar Jan 16 '14 12:01 iceydee

No. Because no sane person does

if (cat)
    meow();
else 
    woff();

or at least, I don't think we should allow it.

KATT avatar Jan 16 '14 12:01 KATT

I don't mind your example. If the return is the only thing that is happening.

Must be very clear that this is the only case where we allow no braces though.

iceydee avatar Jan 16 '14 12:01 iceydee

In theory I agree, in practice I don’t.

No edge cases is better than one edge case.

svoctor avatar Jan 16 '14 13:01 svoctor

In blocks for instance, basically any case an edge case according to style guides:

There are several appropriate style rules, depending on how long the block is:

  • If the block can fit on one line, no wrapping is necessary.
  • If it has to wrap, the closing brace should line up with the first character of the line on which the block is declared. Code within the block should be indented four spaces.
  • If the block takes no parameters, there are no spaces between the characters ^{. If the block takes parameters, there is no space between the ^( characters, but there is one space between the ) {characters.

KATT avatar Jan 16 '14 14:01 KATT

I think we should allow this edge case, it's really handy in blocks, e.g in LR:

typeof (self) __strong strongSelf = weakSelf;
if (!strongSelf) return;

alexfish avatar Jan 16 '14 14:01 alexfish

:+1:

Return early this way is ok for me as well.

Note. I don't like returning early inside if-else branches though, should have a variable and return that at the end

madhikarma avatar Jan 16 '14 14:01 madhikarma

@KATT The blocks example isn't a very good one since we've just put in writing exactly how XCode wants to format it.

iceydee avatar Jan 16 '14 14:01 iceydee

XCode is an edge case.

KATT avatar Jan 16 '14 14:01 KATT

Any progress on this?

alexfish avatar Mar 27 '14 10:03 alexfish

Seemed like a majority was in favour, do a pull request?

KATT avatar Mar 27 '14 11:03 KATT

yeah go for it

alexfish avatar Mar 27 '14 11:03 alexfish

Be very clear about when this is acceptable though.

OSX and iOS has had non-functioning SSL because of non-bracket if-clauses. We must never let those in.

iceydee avatar Mar 27 '14 11:03 iceydee

They separated the condition and action on multiple lines, it wouldn't happen using the above example.

Their bug:

if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
    goto fail;
// [there probably was an if-statement here previously]
    goto fail;  /* MISTAKE! THIS LINE SHOULD NOT BE HERE */
if ((err = SSLHashSHA1.final(&hashCtx, &hashOut)) != 0)
    goto fail;

If they followed the above convention they would've deleted the goto fail when deleting the if-statement too.

:smiley_cat:

(yes, you have a good point)

KATT avatar Mar 27 '14 11:03 KATT

Maybe the condition is too edgy to be in the style guides. Opinions?

KATT avatar Mar 27 '14 12:03 KATT