views:

459

answers:

6

Maybe simpler isn't the proper word; terse might do as well. I'm well-aware that Objective-C is a verbose language; I'm just wondering if it has to be as verbose as it is in the solution I'm showing below. I'm relatively new to the language.

This piece of code counts the number of each type of field in a record, pursuant to generating a variable-height UITableViewCell containing one label per field. My question isn't about how to do this -- I've already figured that out -- I'm just wondering: isn't there a simpler or less verbose way of doing this in Objective-C, using an NSDictionary object? Is this as terse as it gets using an NSDictionary? Solutions using other collection-type objects are also welcome.

Here's my -PartsRecord.countFields method. I've simplified the code slightly.

-(NSDictionary*) countFields {  
NSDictionary* dict = [[NSDictionary alloc] initWithObjectAndKeys:  
     @"s", [NSNumber numberWithUnsignedInt: [self.struts count]],  
     @"h", [NSNumber numberWithUnsignedInt: [self.headAssemblies count]],  
     @"l", (self.leftVent == nil) ?   [NSNumber numberWithUnsignedInt: 0] :  
                                      [NSNumber numberWithUnsignedInt: 1],  
     @"r", (self.rightVent == nil) ?  [NSNumber numberWithUnsignedInt: 0] :  
                                      [NSNumber numberWithUnsignedInt: 1],  
     nil ];  
return dict;

}

Any errors in the above code are errors in transcription , not actual errors in the code.
TIA,
Howard

+3  A: 
@"l", [NSNumber numberWithUnsignedInt: self.leftVent  ? 1 : 0],
@"r", [NSNumber numberWithUnsignedInt: self.rightVent ? 1 : 0],

will shorten it by 2 lines

cobbal
Much cleaner! Thanks. Just goes to prove that despite 3 or so years working in the C/C++ world, there's always a better stylistic variation than what one is used to.
hkatz
Nice tip, and points for style. (I love the ternary operator.) Don't forget to fix the memory leak — that's by far a bigger concern than style. ;-)
Quinn Taylor
+2  A: 

You could also change [[NSDictionary alloc] initWithObjectsAndKeys:] to [NSDictionary dictionaryWithObjectsAndKeys:].

This would be a good idea anyway, since your code is allocating an NSDictionary without releasing it. -dictionaryWithObjectsAndKeys: returns an autoreleased dictionary.

Matt Ball
Alternatively the return statement should be changed to "return [dict autorelease]", which is effectively identical, just more verbose.
sebnow
Right. I just went for the manually released variant since I'm on the iPhone and am generating 1000's of these things.
hkatz
Actually, this raises another question. I'm actually releasing the dictionary in the method that called this one. Is that considered bad form somehow?
hkatz
from the memory management programming guide, "Within a given block of code, the number of times you use copy, alloc and retain should equal the number of times you use release and autorelease" return [dict autorelease]; is the preferred method
cobbal
The fact that you're creating thousands of them is irrelevant inside this method — you have to return an autoreleased object anyway, and as Matt points out, the code you posted leaks the dictionary every time. I also prefer to create the dictionary as autoreleased to begin with — I personally find it more succinct. If you return an autoreleased object, you don't want to release it again outside the method. If you're calling this method a LOT inside a loop, you might consider creating an autorelease pool around the loop to reclaim the memory earlier. Just be aware that there is an overhead hit.
Quinn Taylor
A: 

-[NSNumber numberWithInt: 0] and -[NSNumber numberWithUnsignedInt: 0] are the same number (in fact, they may even be the same object on some implementations of Cocoa).

Graham Lee
Since they only contain 1 or 0, numberWithBool: might be even better.
dreamlax
Uhh there's only one "implementation" of Cocoa - Apple's. Cocoa is an implementation of OpenStep. I assume that's what you meant.
sebnow
No it isn't. The Cocoa framework changes between mac os x releases, and only some of those (which all implement the Cocoa API) have cached common NSNumber instances.
Graham Lee
+3  A: 

Unless you're intending to key on the numbers, your object ordering is wrong. It should be object then key.

dict = [NSDictionary dictionaryWithObjectsAndKeys:
   [NSNumber numberWithUnsignedInt: [self.struts count]], @"s",
   [NSNumber numberWithUnsignedInt: [self.headAssemblies count]], @"h",
   [NSNumber numberWithUnsignedInt: self.leftVent  ? 1 : 0], @"l", 
   [NSNumber numberWithUnsignedInt: self.rightVent ? 1 : 0], @"r",
   nil ];

This way, calling [dict objectForKey:@"s"] would return the [self.struts count] number object.

Ashley Clark
good point, since I'm fairly sure keys have to be strings
cobbal
Keys can be NSObject derived object, as can the values.
dreamlax
You're right about my having the order wrong: I'm keying on the strings.
hkatz
@cobbal, any object that conforms to NSCopying and whose hash value is constant while in the collection can be a key.
Ashley Clark
+1  A: 

Does it have to be a dictionary? If not, perhaps resorting to some plain C might be easier to comprehend.

typedef struct {
    unsigned strutCount;
    unsigned headAssembliesCount;
    BOOL leftVent;
    BOOL rightVent;
} Fields;


- (void) countFields:(Fields *) fields
{
    fields->strutCount = [self.struts count];
    fields->headAssembliesCount = [self.headAssemblies count];
    fields->leftVent = (self.leftVent != nil);
    fields->rightVent = (self.rightVent != nil);
}
dreamlax
A: 

I think that using macros are quite ok to simplify typing and increase the readability

#define SET_KEY_NUMBER( _key, _value ) \
  [NSNumber numberWithUnsignedInt:_value], _key
#define SET_KEY_BOOL( _key, _bool ) \
  [NSNumber numberWithUnsignedInt: (_bool) ? 1 : 0], _key

-(NSDictionary*) countFields {  
    NSDictionary* dict = [NSDictionary dictionaryWithObjectsAndKeys: 
      SET_KEY_NUMBER( @"s", [self.struts count] ),
      SET_KEY_NUMBER( @"h", [self.headAssemblies count] ),
      SET_KEY_BOOL(   @"l", self.leftVent == nil ),
      SET_KEY_BOOL(   @"r", self. rightVent == nil ),
      nil ];  
    return dict;
}

...and @Matt Ball is correct, when returning a object it should be autoreleased

epatel
Macro names like that belong in Windows API ;)
dreamlax
In my opinion this just makes it less readable.
sebnow
Man, that hurt, I'm a Mac! :)
epatel
I won't say that belongs in Windows, it just seems like it belongs in raw C/C++. The big downside of using macros is debuggability. Since the compiler replaces the macro call with the #defined text, the source and binary won't line up. I've written some pretty gnarly macros in my day, and my rule of thumb is to avoid them if possible. I only use them when they save me a *significant* amount of work, are sufficiently self-evident, and I can painstakingly guarantee their correctness. Even then, I reconsider whether it's really a good idea... :-)
Quinn Taylor