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?
8
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.
- Guard checks
- 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.
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.
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 ifelse 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.