14

I am just curious if I am doing this right.

NSString *fileContents;    
NSError *fileError = nil;

fileContents = [[NSString stringWithContentsOfFile:fileOnDisk
                          encoding:NSMacOSRomanStringEncoding
                          error:&fileError] retain];

if(fileError != nil) {
    NSLog(@"Error : %@", [fileError localizedDescription]);
}

// Other Code ...
[fileContents release];

.

EDIT (to reflect bbums comments)

.

NSString *fileOnDisk = @"/Users/Gary/Documents/Xcode/RnD/Maya.MEL";
NSError  *fileError; // Should this be *fileError = nil;
NSString *fileContents;
int       status = 0;

fileContents = [[NSString stringWithContentsOfFile:fileOnDisk
                          encoding:NSMacOSRomanStringEncoding
                          error:&fileError] retain];

if(fileContents == nil) {
    NSLog(@"FileError: %@", [fileError localizedDescription]);
    status = 1;
} else {
    NSLog(@"Success  : %@", fileContents);
}

// Clean up
[fileContents release];
[pool drain];
return status;

gary

fuzzygoat
  • 26,573
  • 48
  • 165
  • 294

1 Answers1

48
NSError *fileError = nil;
....
if(fileError != nil)
....

That is incorrect. You must not assume anything about the return-by-reference value of fileError until you check whether or not fileContents was nil. Not ever. Setting fileError to nil prior to calling the pass-error-by-reference method does nothing useful.

That is, your code should read (fixed now that I'm no longer running from plane to plane and hopping on WiFi in between connections...):

NSString *fileContents;    
NSError *fileError;

fileContents = [[NSString stringWithContentsOfFile:fileOnDisk
                          encoding:NSMacOSRomanStringEncoding
                          error:&fileError] retain];

if(fileContents == nil) {
    NSLog(@"Error : %@", [fileError localizedDescription]);
    // ... i.e. handle the error here more
    return ...; // often returning after handling the errors, sometimes you might continue
}

// Other Code ...
[fileContents release];
bbum
  • 162,346
  • 23
  • 271
  • 359
  • 7
    bbum, did you mean `if(fileContents == nil) {` ? – Todd Ditchendorf Nov 27 '09 at 19:22
  • 5
    This is described in Apple's documentation here: http://developer.apple.com/mac/library/documentation/cocoa/Conceptual/ErrorHandlingCocoa/CreateCustomizeNSError/CreateCustomizeNSError.html#//apple_ref/doc/uid/TP40001806-CH204-SW2 – Johan Kool Nov 27 '09 at 19:32
  • Thanks bbum, I can see where your going there, and I can see how it now makes more sense to check the fileContents for nil. However, is it not the same difference, don't you only get an error if fileContents is nil. Unless of course there are situations where fileContents is not nil and their is an error. Thanks for the tip. – fuzzygoat Nov 27 '09 at 21:35
  • Also in your new version do you still need to set NSError *fileError = nil; it seems a bit pointless as your now checking fileContents? COuld you have NSError *fileError; instead? – fuzzygoat Nov 27 '09 at 21:38
  • 3
    Edited to fix my rushed answer. Sorry about that. Yes -- no need to initialize fileError because you *never* read it *unless* you first check the return value of the method to determine that an error actually happened. – bbum Nov 27 '09 at 22:31
  • bbum, no problem your answer got me on the right track. Many thanks for your time & help. – fuzzygoat Nov 28 '09 at 11:35
  • @bbum, i know that is an old answer, but why you put a "retain" for the fileContents. It is in "autorelease" state. I think that "retain" and "[fileContents release];" is an extra code. are your right with me ?thanks – samir Sep 28 '12 at 08:17
  • Copy/paste from OP's code. Didn't remove the extraneous retain/release. – bbum Sep 28 '12 at 16:32