Friday, April 5, 2013

Fun with BOOLs

I ran into a fun bug today involving BOOLs and thought I'd share.

I have a UISwitch that is enabled and disabled programmatically based on some conditions. The initial version of the code looked kind of like this:


- (void) updateUI {
    self.aSwitch.enabled = [self shouldEnableSwitch];
}

- (BOOL) shouldEnableSwitch {
    id obj1 = ...
    id obj2 = ...
    // only enable switch when both objects are non-nil
    return (obj1 && obj2);
}

This code was working correctly until some refactoring work. The refactoring eliminated the need for one of the condition objects (obj2) and the new version of the shouldEnableSwitch method looked like this:

- (BOOL) shouldEnableSwitch {
    id obj1 = ...
    // only enable switch when obj1 is non-nil
    return (obj1);
}

This broke the UISwitch. It was initially disabled and would not get enabled later when the required condition was met.

So what happened?

The first problem is that the project currently has the "-Wint-conversion" warning suppressed (need to fix that) so I didn't get the help from the compiler I should have. 

Had that warning been enabled I would have seen this error:

warning: incompatible pointer to integer conversion returning '__strong id' from a function with result type 
'BOOL' (aka 'signed char') [-Wint-conversion]

This tells us that we are returning an object reference instead of a BOOL. 

Remember that a BOOL is really just an int (signed char). By returning an object reference we end up storing the address of the object inside the BOOL variable (or 0 if the reference is nil).

Seems like it should still work though right? 

Isn't a nil / zero value always false and a non-nil / non-zero value always true? Well, kind of. That is true when used as the condition inside an if statement like this:

int i = 12893;
if (i) {
   NSLog(@"yup");
}

But there is another way to evaluate BOOLs. You can compare a BOOL to the YES and NO constants that Objective-C defines.

Guess what happens when this code runs:

BOOL b = (BOOL) 12345;

if (b == YES) {
   NSLog(@"b is YES");
} else if (b == NO) {
  NSLog(@"b is NO");
}  else {
  NSLog(@"b is neither YES nor NO! What?");
}

Objective-C defines YES as 1 and NO as 0. Since a BOOL is an int (signed char) it can hold a lot more values than just YES and NO.

UISwitch is likely comparing the argument I pass to one of these constants or the current BOOL value. Since one or both of them have a value other than 1 or 0 it is failing.

The fix is to simply ensure we return a BOOL rather than an object reference:

- (BOOL) shouldEnableSwitch {
    id obj1 = ...
    // only enable switch when obj1 is non-nil. Return a BOOL not an object reference.
    return (obj1 != nil);
}

The complete fix is to also make sure all warnings are enabled and treated as errors. 


No comments: