views:

162

answers:

4

Just trying a few things out and noticed that this works, it does compile, but I just wanted to check if it would be considered good practice or something to be avoided?

NSString *fileName = @"image";
fileName = [fileName stringByAppendingString:@".png"];
NSLog(@"TEST  : %@", fileName);

OUTPUT: TEST  : image.png

Might be better written as:

NSString *fileName = @"image";
NSString *tempName;
tempName = [fileName stringByAppendingString:@".png"];
NSLog(@"TEST  : %@", tempName);

just curious.

+5  A: 

The first line is declaring an NSString literal. It has storage that lasts the lifetime of the process, so doesn't need to be released.

The call to stringByAppendingString returns an autoreleased NSString. That should not be released either, but will last until it gets to the next autorelease pool drain. So assigning the result of the the stringByAppendingString call back to the fileName pointer is perfectly fine in this case. In general, however, you should check what your object lifetimes are, and handle them accordingly (e.g. if fileName had been declared as a string that you own the memory to you would need to release it, so using a temp going to be necessary).

The other thing to check is if you're doing anything with fileName after this snippet - e.g. holding on to it in a instance variable - in which case your will need to retain it.

Phil Nash
+1  A: 

I think you're right this is really down to preferred style.

Personally I like your first example, the codes not complicated and the first version is concise and easier on the eyes. Theres too much of the 'language' hiding what it's doing in the second example.

As noted memory management doesn't seem to be an issue in the examples.

Neil Foley
+2  A: 

The difference is merely whether you still need the reference to the literal string or not. From the memory management POV and the object creational POV it really shouldn't matter. One thing to keep in mind though is that the second example makes it slightly easier when debugging. My preferred version would look like this:

NSString *fileName = @"image";
NSString *tempName = [fileName stringByAppendingString:@".png"];
NSLog(@"TEST  : %@", tempName);

But in the end this is just a matter of preference.

tcurdt
+1  A: 

Internally, compilers will normally break your code up into a representation called "Single Static Assignment" where a given variable is only ever assigned one value and all statements are as simple as possible (compound elements are separated out into different lines). Your second example follows this approach.

Programmers do sometimes write like this. It is considered the clearest way of writing code since you can write all statements as basic tuples: A = B operator C. But it is normally considered too verbose for code that is "obvious", so it is an uncommon style (outside of situations where you're trying to make very cryptic code comprehensible).

Generally speaking, programmers will not be confused by your first example and it is considered acceptable where you don't need the original fileName again. However, many Obj-C programmers, encourage the following style:

NSString *fileName = [@"image" stringByAppendingString:@".png"];
NSLog(@"TEST  : %@", fileName);

or even (depending on horizontal space on the line):

NSLog(@"TEST  : %@", [@"image" stringByAppendingString:@".png"]);

i.e. if you only use a variable once, don't name it (just use it in place).

On a stylistic note though, if you were following the Single Static Assigment approach, you shouldn't use tempName as your variable name since it doesn't explain the role of the variable -- you'd instead use something like fileNameWithExtension. In a broader sense, I normally avoid using "temp" as a prefix since it is too easy to start naming everything "temp" (all local variables are temporary so it has little meaning).

Matt Gallagher