-2

I have a custom designated initialiser like this:

- (id)initWithLocation:(CLLocation *)location
{
    if (location == nil)
    {
        return nil;
    }

    self = [super init];

    if (self)
    {
        self.location = location;
    }

    return self;
}

My colleagues sometimes ask me to wrap everything (apart from the return), that follows after first curly brace pair, in an else clause:

- (id)initWithLocation:(CLLocation *)location
{
    if (location == nil)
    {
        return nil;
    }
    else
    {
        self = [super init];

        if (self)
        {
            self.location = location;
        }

        return self;
    }
}

But I am not sure. What is the point? The first if condition is a quick death check. If the parameter is not provided, no instance is provided. This serves as enforcement for providing a parameter. else seems superficial here as it is implied in the flow of the code.

What would be the advantage having it?

Martijn Pieters
  • 14,604
Earl Grey
  • 628
  • Guys, this has nothing to do with how return is implemented! Please note that I explicitly say that return is not wrapped in else clause and I follow the single return at the end of method principle. – Earl Grey Nov 19 '13 at 11:53
  • 1
    clarify your problem with understandable code example, showing what exactly your colleague asks – gnat Nov 19 '13 at 11:54
  • @gnat, IMHO the question needs no additional clarification. The examples are there, and the text is clear as well. – superM Nov 19 '13 at 11:57
  • @superM as far as I can tell, asker objects against the example introduced in recent suggested edit: "note that I explicitly say that return is not wrapped in else clause and I follow the single return" - that's why I asked for clarification – gnat Nov 19 '13 at 12:00
  • @gnat, it's a counter-example of what the OP is asked to do. – superM Nov 19 '13 at 12:09
  • @superM in prior comment, OP wrote "I follow the single return at the end of method principle" - none of the examples is like that – gnat Nov 19 '13 at 12:10
  • This is complete mess.
    1. The code is extremely trivial. What could be clarified here is beyond my understanding.
    2. The possible modification I ask about (and which constitutes the merrit of my question) has NOTHING TO DO with where I possibly would handle what is returned. My question is about logical flow of code.
    3. The second explicit example in CodeCaster's revision ruins my question as he put the return into ELSE in spite of my stating in the original question that it is not part of ELSE.
    4. I get downvote because other people can't read code/change the question with faulty revisions.
     – Earl Grey Nov 19 '13 at 12:13
  • 3
    Then please update the question yourself with what changes your colleague suggested, as we can't guess that. Your question originally stated "My colleagues sometimes ask me to wrap everything (except return), that follows after first curly brace pair, in an ELSE clause.", which doesn't mention a return variable, but I must admit I missed that "except return" part. Also, I don't speak Obj-C but you probably can't set self to nil her eanyway. Please remember that if the question was clear, nobody would be asking for clarification, so show what you actually mean! :) – CodeCaster Nov 19 '13 at 14:59

3 Answers3

5

I think that what you and your colleague are discussing is expressing intent within your code.

There are two broad cases that we need to consider to answer your question.

  1. Guard checks
  2. Separation of application logic / conditional branching

Guard Checks
Your example demonstrate the first case - a guard check. A guard check inspects inputs or other requisite inputs to make sure things are valid. If things aren't valid then the function needs to immediately exit or return.

The conventional wisdom that I have encountered is that guard checks don't really count as part of the logic of your function. An experienced programmer skimming your code would take note of the guard checks and then start paying attention at the initialization step in your example.

So in the case of the guard checks, you don't really need the else {} block to clearly express the intent of your function.

Conditional Branching
The other case when there is extensive application logic requires a different consideration in order to express intent. The simplified version looks like this: 

if( A )
{
    ...
    doFoo()
}
else 
{
   doBar()
}

Concerns about expressing intent now revolve around how much code those ellipses represent. If it's "a lot" of code, then your colleague is correct and you need to wrap the not A branch with an else statement and potentially its own set of braces. The thinking here is that you don't want future maintainers to have to deal with too much mental overhead while trying to understand your function. The else clearly demonstrates you have two main paths.

So to answer your question, you have to ask which option more clearly expresses your intent as the developer of the code.

4

What would be the advantage having it?

None whatsoever, in my opinion. It adds an unnecessary level of nesting while it shouldn't be hard to interpret the "fail fast" if (foo == nil) return statement(s) on top of a method.

CodeCaster
  • 2,394
1

The additional else statement will have no impact on logic or performance. But it still has its advantages.

First of all, it will increase readability by showing a clear chain of decisions. Going further, you could declare a variable in the beginning of the initializer to hold the return value. Then in the main part you could edit the variable and/or assign it a new value. The last line would be the return statement. This will increase readability even more.

Another advantage is that the if\else design allows you to track the mutual-exclusive options. If you'll need to add another option, such style will help you to quickly place it in the right place without additional code changes and without a risk of errors while implementing the new logic.

superM
  • 7,383