Declare at start, or as you go?

Question

I would say this is pretty much a style / readability thing, although I do see nearly all objective-c/cocoa formatted as per "METHOD_002". I am just curious if "METHOD_001" would be considered bad style, there are advantages to having all the declarations at the top of a method, but then again there are disadvantages in terms of readability if your not declaring objects where using them?

METHOD_001

-(IBAction)dateButtonPressed {

    NSDate      *dateSelected;
    NSString    *dateString;
    NSArray     *dateItems;
    NSString    *alertMessage;
    UIAlertView *alert;

    dateSelected = [datePicker date];
    dateString = [[NSString alloc] initWithFormat:@"%@", dateSelected];
    dateItems = [dateString componentsSeparatedByString:@" "];
    alertMessage = [[NSString alloc] initWithFormat:@"Date: %@ Time: %@",  [dateItems objectAtIndex:0], [dateItems objectAtIndex:1]];
    alert = [[UIAlertView alloc] initWithTitle:@"You Selected" 
                                       message:alertMessage 
                                      delegate:nil 
                             cancelButtonTitle:@"OK" 
                             otherButtonTitles:nil];
    [alert show];
    [alert release];
    [alertMessage release];
    [dateString release];

}

METHOD_002

-(IBAction)dateButtonPressed {

    NSDate *dateSelected = [datePicker date];
    NSString *dateString = [[NSString alloc] initWithFormat:@"%@", dateSelected];
    NSArray *dateItems = [dateString componentsSeparatedByString:@" "];
    NSString *alertMessage = [[NSString alloc] initWithFormat:@"Date: %@ Time: %@",  [dateItems objectAtIndex:0], [dateItems objectAtIndex:1]];
    UIAlertView *alert = [[UIAlertView alloc] initWithTitle:@"You Selected" 
                                                    message:alertMessage 
                                                   delegate:nil 
                                          cancelButtonTitle:@"OK" 
                                          otherButtonTitles:nil];
    [alert show];
    [alert release];
    [alertMessage release];
    [dateString release];

}

gary

Answer 1

I would tend to favour "METHOD_002". It seems more concise and easier to read, although that may just be because I'm used to it.

I suppose the advantage with "METHOD_001" is that if you decide to promote a local variable to an instance variable, you simply have to delete its local declaration at the top of the method rather than hunting for its declaration. I can only foresee this becoming a problem for very very large methods however.

Answer 2

Definitely METHOD_002. I was taught METHOD_001 back in the day when I wrote long, long, long blocks of ASP code and it seemed to make sense then to list all variables on the top, like a warning (on this journey we will encounter the following ...). In practice, however, one would stare at something called strFwdBck and wonder what was in the coffee that particular day. We didn't have "unused variable" warnings back then, at least not that I was aware of.

Now I make a point of only declaring what I really need and using it immediately. And even then, I often go back and tighten things up.

-(IBAction)dateButtonPressed {
     NSString *dateString = [[NSString alloc] initWithFormat:@"%@", [datePicker date]];
     NSArray *dateItems = [dateString componentsSeparatedByString:@" "];

     NSString *alertMessage = [[NSString alloc] initWithFormat:@"Date: %@ Time: %@",        [dateItems objectAtIndex:0], [dateItems objectAtIndex:1]];
    [dateString release];

    UIAlertView *alert = [[UIAlertView alloc] initWithTitle:@"You Selected" 
                                                    message:alertMessage 
                                                   delegate:nil 
                                          cancelButtonTitle:@"OK" 
                                          otherButtonTitles:nil];
    [alert show];
    [alert release];
    [alertMessage release];
}

Answer 3

I usually mix the two. If the variable is assigned only once, then I use method_002. This keeps the declaration right where the variable is used. However, if the variable is assigned multiple times I use method_001 because then I don't have to hunt through the code to find the definition.

Example of the latter case:

Using method_002:

-(void) someMethod{
    //...some code
    SomeClass *myVar=[assign someValue];
    //...some more code
    if (someTest) {
        myVar=someValue;
    }else{
        myVar=someOtherValue;
    }
}

If you come back to the code and look at myVar=someOtherValue; you will have to snake your way back up the code to find the definition. Conversely if you use method_001 you get:

-(void) someMethod{
    SomeClass *myVar;
    //...some code
    myVar=[assign someValue];
    //...some more code
    if (someTest) {
        myVar=someValue;
    }else{
        myVar=someOtherValue;
    }
}

And you just jump to the top to see the definition.

If you use this style consistently, you will know automatically that any variables defined at the start of a function/method will have multiple assignments and any defined in the body do not.

This really helps comprehension, especially if you spend a lot of time having to go back to old code you've completely forgotten.

Answer 4

METHOD_002 definitely. While some might come up with justifications for METHOD_001, it exists primarily for technical, historical reasons. Older C compilers needed all the stack variables (local variables) defined before any executable code. This is part of the ANSI standard. It no longer is, and should be relegated to history.

METHOD_002 has several advantages:

• It encourages block scoping. By defining a variable inside of a block (a for() loop for instance), that variable cannot be accidentally used outside of its meaningful scope. ANSI allows block scoping by defining variables at the top of the block, but I find that it is seldom done this way in practice once people get used to declaring at the top of the function.

• It catches other accidental re-use. For instance, you define a variable "result" at the top of the function and assign it to nil. Your code assumes that it is NULL at the start of some loop. Then you insert more code at the top of the function, and through habit, reuse the variable "result." You may have just created unintended side effects for the later code. If you use METHOD_002, then you'll either have made the former result block-local, or you'll get a compiler error when you redeclare another variable with the same name in the same scope.

• It makes the Extract refactor much easier, since variable declarations will generally be close to the code that uses them. Extraction will also be much less likely to have unintended side-effects, because variables are less likely to be reused in distinct areas of the code.

• It reduces the likelihood of "cruft." Variables at the top of the function are very seldom deleted when the code that uses them is deleted. The compiler can optimize this away, but it's still code cruft.

• It encourages more meaningful names through less variable reuse. In practice, I find that when people declare variables at the top of the function, they are more likely to just reuse some variable called "tmpValue" for several different uses through the function. There's no reason this has to be true, but I find that the further the variable declaration is from the code that uses it, the less likely people will take the trouble to declare a new variable. There are few things that are better at preventing bugs than good naming.

There is one disadvantage that I find with METHOD_002:

• It is harder to find the type of a variable. This is less significant with an IDE, since you can generally easily jump to its declaration. And I argue that the name of the variable should generally make its type obvious. But still, it can sometimes be difficult to find the type. As a corollary, it can sometimes make it difficult to determine the scope of a variable.

Answer 5

There are good arguments for either style, but I would suggest going with whatever you're used to doing, or more importantly, whatever format your clients and/or employers use.

Whatever you choose for your personal projects, just do it consistently- consistency helps quite a bit when you're trying to avoid introducing annoying/silly syntax errors. (In the absence of external guidance, I use google's style guide)

Answer 6

I think that your method 2 is really 1/2 way between method 1 and what I would call method 3

method 3 would be where you declare variables wherever you are in the code, right before they are used the first time. If that's at the end of the function so be it.

The difference between method 1 and 2 in your examples seems to be mostly a matter of initialization. I think that initting variables when you declare them is a good idea, and makes for readability. However, I also tend to think that declaring variables at the top makes it easier to debug. If you're not reading the function top to bottom, it can be tricky to figure out where the variable was declared, and what type it might be.

This mostly becomes a problem when you have long functions (Which you shouldn't be doing anyway, but it ends up happening) and so 20 lines into a function you declare a variable, and then use it again 50 lines after that and again 20 lines later. Figuring out what type it was is easier if you just go to the beginning of the function, and look there...

However, as you noted this is a style issue, so ends up being a matter of opinion...