Next article: SocketObjC: A Networkable Messaging Library
Previous article: iPhone Apps I Can't Have
Tags: advice apple cocoa fridayqna
Welcome back to another Friday Q&A. This week, Quentin Carnicelli (who is heavily involved in generating my paychecks) has suggested that I talk about things that every Apple programmer should know. In other words, common Cocoa design and implementation decisions that I'd prefer Apple not to make.
But First, a Brief Word from Our Sponsor
Before I get into the meat of the post, I have a bit of meta-business. While I greatly enjoy writing Friday Q&A and am humbled by the great feedback I get about it, it also represents a significant drain on my time that's becoming hard to sustain. Therefore, starting with this post, I will be scaling Friday Q&A back to a biweekly schedule. I'll be writing the same stuff in the same place, just not quite as often. I hope that what I'm taking away in quantity, I'll be able to make up for in quality.
And Now, Back to Our Show
While the theme of this article is common problems in Cocoa, I think this will also have a great deal of relevance to any Cocoa programmer as well. Ultimately, this will be a collection of common problems in API design and implementation, and large chunks of any real application count as APIs. While I really want Apple to follow these ideas, if you follow them as well, they should make your life much easier.
Without further ado, the list.
Always use [self class] when invoking your own class methods
A bunch of these boil down to making your code easier to subclass, and this is the first. If you hardcode your class when invoking a class method, it makes it impossible for a subclass to override that behavior. If that class method is public, then you've created a frustrating situation: the method exists, is published, can be overridden, but won't be called if you do, so you can't change the behavior.
Don't access instance variables directly if there's an accessor
This is much like the previous one. If there's an accessor, there's an expectation that you can override it in a subclass to alter the value which is used. If you access the instance variable directly, you bypass that override, making it impossible for subclasses to alter behavior. Even worse, if you use the accessor only sometimes, then you get bizarre inconsistent behavior that's easy to go wrong. Apple really loves to do this one, so as a user of Cocoa, be careful when subclassing and overriding accessors, and be prepared for them not to be called.
If components of functionality are exposed as public methods, the implementation should always call them when it needs that functionality
Yet another ease-of-subclassing item. Sometimes Apple provides a public method, but doesn't use it internally to accomplish the task it's built for. An example of this is NSSliderCell, which provides a drawBarInside:flipped: method, but which does its own bar drawing separately. If you want custom bar drawing, you either have to override a higher level drawing method, or you have to override the private _usesCustomTrackImage method to convince it to call your custom code. Another example is NSMutableURLRequest's setHTTPBodyStream: method. If you create a custom NSInputStream class and pass an instance of that custom class as the parameter, it won't work. You have to override the private _scheduleInCFRunLoop:forMode: method for the two components to work together. Subclasses should be able to override a public method to alter functionality, or work with existing APIs, without jumping through these hoops.
Don't write empty stub methods and then make them public
NSView has this nifty method:
- (BOOL)lockFocusIfCanDrawInContext:(NSGraphicsContext *)context;
It baffles me as to why this method is public. It's useless, so why have it at all? If they plan to implement it in the future, they could make it public once it's implemented. I can only assume that it was implemented as a stub with the intent to complete it, but then it slipped through the cracks.
Apple doesn't do this much, but suffice it to say that it shouldn't happen at all.
User data fields should be id
It's a common pattern to have "user data" as part of an API, which is just some arbitrary data which can be passed through a callback or attached to an object. Sometimes this is an object, but sometimes Cocoa presents user data as a void *. This greatly complicates memory management (especially when using garbage collection) to little benefit. The common case is to pass an object as user data, and the API should simplify that. For the rare cases where you want a different type of pointer, the programmer can always wrap it in an NSValue.
Make up your mind on what "thread safe" means
As I discussed in an earlier post, Apple is pretty inconsistent about what "thread safe" actually means. Sometimes it means any instance can be safely used from multiple threads simultaneously. Sometimes it means any instance can be used from one thread at a time, but that you must synchronize access. Sometimes that's described as "not thread safe" instead. It's tremendously confusing! The world is becoming heavily multithreaded, and we need more explicit descriptions of what these APIs require.
Make fewer APIs dependent on the main thread
The main thread is a huge bottleneck in a modern Cocoa app, because many APIs only work on the main thread. Most GUI manipulation can't be safely done off the main thread, even if you take care to cleanly keep each window on a single thread. Entire APIs, like WebKit, can only be used from the main thread. Fortunately, the situation is gradually improving.
Make every runloop API take a modes parameter
Do a google search for webview modal to see what the problem is here. WebView depends on a running runloop to do its processing, but it only runs in the default mode. If you want your WebView to remain functional while a modal window is running, you're out of luck. You should be able to easily schedule the WebView in the modal runloop mode, but the API isn't there, so you can't. While it's perfectly reasonable to schedule into one particular runloop mode by default, a runloop-dependent API should always provide for the ability to schedule on other modes too.
Make it easy to convert between different classes with similar capabilities
An example of this is NSImage and CGImage. Before Snow Leopard, there was no easy way to convert between the two, even though they were fairly similar. (Yes, NSImage can contain multiple representations, isn't necessarily pixel-based, etc. They're still conceptually close.) If you had one, and needed the other, it was a bunch of work to go from one to the other.
In 10.6, Apple added APIs to NSImage to make it easy to convert between them, and suddenly life became a lot easier.
Toll free bridging in CoreFoundation is the pinnacle of how to do this right. The objects are interoperable, and just require a cast to convince the compiler that you're not insane. While this isn't always possible, easy methods to do explicit conversion help enormously.
There are still areas where this is lacking. NSColor and CGColor are difficult to convert between. CFBundle and NSBundle are extremely similar but not interchangeable or convertible.
Don't pollute namespaces
One of the unfortunate things missing from Objective-C is namespaces. It's all too easy for components in a framework to conflict with components in an application. If you're building a framework, then you need to minimize the possibility of this happening as much as possible.
Always prefix class names with something that should be reasonably unique. This goes even for private classes. They can conflict and cause problems even though they're never exposed as part of your public API.
You also need to prefix category methods that aren't made public. If they aren't prefixed, there's a risk of conflict with other category methods. The same also goes for private methods.
Conclusion
Overall, Cocoa is a great API, but there are a few common problems which make things a little bit less smooth than they otherwise could be. The intent is not to bash Apple, but just show how things could be made a little bit better, and give some ways that the rest of us can make our own APIs better as well.
That's it for this edition of Friday Q&A. Again, the next one will now be in two weeks. Until then, keep sending in your ideas for posts. Friday Q&A is driven by user ideas, so if you have a topic you'd like to see covered here, get in touch.
Comments:
#ifdef __OBJC__
typedef NSData TFBDataRef;
#else
typedef CFDataRef TFBDataRef;
#endif
The C/C++ typedef for id is a bit tricky, but the actual C-language underlying typedef for an id is "struct objc_object".
Overall, this works rather nicely when bridging between Obj-C code and C/C++ code, and avoids the need for casts.
When I come across a problem in the APIs, I look at the tradeoff between the time and effort it takes to file a decent bug, the benefit to me if it results in action, and the probability of a fix. Most of the time, the time and effort to file a bug isn't worth it. For most little stuff, if you're lucky, it gets fixed years later and with no notice. Usually it's just ignored.
I thought I had read somewhere that if you're using automatically generated accesors, you should not use them inside init:(even if you use them everywhere else)
Do you directly access instance variables in your init method, or not even there?
First such order of business: make NSXMLParser read 4KB-chunks from a stream, so it's actually usable on serious data on the iPhone. All of about 5 lines to change, 20 to add, all internal. *sigh*
Now, if you turn out to be the only one that files a bug on an API, it may not get the traction you like. But if 20, or 30 (or a few hundred) developers are all pushing back on a particular API, I think it helps bring the issue to Apple's attention.
They may still not fix it in a timeframe that is useful to you or me (they may have different priorities), but I think it is worthwhile. They also may solve the problem in a different way (I know I sometimes do this with my own customer bug reports).
I also file new bugs for any major issue that was not addressed with each major release. I think with the help of twitter/blogs, you can get other developers to help raise Apple's attention to their specific needs.
The thing is, it conflicts with the results I've seen, and when there's a conflict between what someone says and what I actually see, I go with what I see.
Even major bugs I've filed like NSOperationQueue have gotten a brush-off and no traction until I started raising a ruckus through other means. For inconsequential bugs, odds are extremely high that nothing will happen. Even if it does, I already put in the effort to work around it, and there's often no benefit, to me, to having a fix once that's done.
I get that filing bugs is useful to Apple, but as far as I've seen it's frequently not worth the time for me. If Apple benefits from my time spent filing bugs, and they want me to file more bugs, maybe they should start paying for my time.
I don't think Apple engineers are lying to me or anything like that. They're just a bit isolated. Most Apple engineers I've talked with don't really know what it's like to be a small third-party developer. It's not their fault, that's just not an environment they're familiar with. Apple engineers largely don't realize that our access to Radar is greatly limited compared to their own, and don't know what it's like to have every bug come back as Duplicate and then never be able to hear anything about it again. And this is a large part of why I don't file many bugs, even though Apple engineers say it's the way to get things done.
I think a better route would be to add a KVO flag checked by your accessor, and decline automatic KVO, so you can have control if you need it.
Or just don't use KVO. I've come across few situations where it's worth struggling with.
If you file bugs, there's a good chance the bugs will get fixed eventually. If not, there's a possibility Apple doesn't even know about the bugs.
@interface Person : NSObject {
NSDate *dob;
NSNumber *age;
}
@property (nonatomic, retain) NSDate *dob;
@property (nonatomic, retain, readonly) NSNumber *age;
@end
@implementation
@synthesize dob;
@synthesize age;
- (void)setDob:(NSDate *)date_ {
[date release];
date = [date_ retain];
age = [self calculateAge];
}
+ (NSSet *)keyPathsForValuesAffectingValueForKey:(NSString *)key
{
NSSet *keyPaths = [super keyPathsForValuesAffectingValueForKey:key];
if ([key isEqualToString:@"age"])
{
NSSet *affectingKeys = [NSSet setWithObjects:@"dob", @"firstName",nil];
keyPaths = [keyPaths setByAddingObjectsFromSet:affectingKeys];
}
return keyPaths;
}
@end
Admittedly, if you want to implement an age setter in the subclass things get hairy, but that would likely be the case even if you used a private setter for age in the above implementation since the Person is likely to have semantic dependencies on age and dob being related, so the only clean way to subclass would be to override all the declared entry points anyway.
Louis Gerbarg: That code doesn't bypass the accessor. There isn't any accessor to bypass. I have no problem with direct ivar access in the absence of an accessor. What I have a problem with is declaring a public accessor and then not using it in the implementation, because it's deceptive for subclassers. If the accessor isn't there in the first place, then it's fine.
@interface Person : NSObject {
NSString *firstName;
NSNumber *lastName;
NSNumber *fullName;
}
@property (nonatomic, retain) NSString *firstName;
@property (nonatomic, retain) NSNumber *lastName;
@property (nonatomic, retain) NSNumber *fullName;
@end
@implementation
@synthesize firstName;
@synthesize lastName;
@synthesize fullName;
//Omitting sanity checking and error code because it would make things a lot longer and just obfuscate the point
- (void)setLastName:(NSDate *)lastName_ {
[lastName release];
lastName = [lastName_ retain];
[fullName release];
fullName = [[NSString alloc] initWithFormat:@"%@ %@", firstName, lastName];
}
- (void)setFirstName:(NSDate *)firstName_ {
[firstName release];
firstName = [firstName_ retain];
[fullName release];
fullName = [[NSString alloc] initWithFormat:@"%@ %@", firstName, lastName];
}
- (void)setFullName:(NSDate *)fullName_ {
[fullName_ release];
fullName = [fullName_ retain];
NSArray *names = [fullName componentsSeparatedByString:@" "];
[firstName release];
firstName = [[names objectAtIndex:0] retain];
[lastName release];
lastName = [[names objectAtIndex:1] retain];
}
+ (NSSet *)keyPathsForValuesAffectingValueForKey:(NSString *)key
{
NSSet *keyPaths = [super keyPathsForValuesAffectingValueForKey:key];
if ([key isEqualToString:@"firstName"]) {
NSSet *affectingKeys = [NSSet setWithObjects:@"fullName", @"lastName",nil];
keyPaths = [keyPaths setByAddingObjectsFromSet:affectingKeys];
} else if ([key isEqualToString:@"lastName"]) {
NSSet *affectingKeys = [NSSet setWithObjects:@"fullName", @"firstName",nil];
keyPaths = [keyPaths setByAddingObjectsFromSet:affectingKeys];
} else if ([key isEqualToString:@"firstName"]) {
NSSet *affectingKeys = [NSSet setWithObjects:@"lastName", @"firstName",nil];
keyPaths = [keyPaths setByAddingObjectsFromSet:affectingKeys];
}
return keyPaths;
}
@end
But the @synthesize generates an accessor, does it not?
Trevor: Since that property is readonly, @synthesize only generates a getter, but the example bypasses the non-existent setter.
setFirstName: calls setFullName: calls setFirstName:....
Avoiding that requires the setters to have additional state hidden somewhere which is just as likely to blow up when subclassing. If you are changing the interaction semantics of dependent properties you will need to follow certain rules in subclass in order to guarantee the invariants the superclass depends on.
In my opinion it is better to just figure out some way to structure appropriate asserts for those invariants in your code (so the mistakes blow up in a spectacular and obvious manner), and document the constraints for subclassing.
For a complex case like this, I could see bending the rule a bit and accessing things directly. Where it really grates is when no such complexity exists, it's just a simple value, but the code accesses the ivar directly and bypasses the accessor for no reason. For example, there are many NSWindow properties which you can't override because NSWindow will bypass the getter.
What I generally do in a case like this (but did not because it would have made the example more complicated) is make all the accessors thin wrappers that funnel through a single common function that updates all of the dependent properties together. That places all the direct manipulation in one easily auditable location, and also provides a method that subclasses can sensibly override that conveys the semantic relation (imagine all the above setters just called something like -[Person setName:nameType:] that handled all the special behavior and directly manipulated the appropriate ivars).
I say: Maybe Apple should take it upon themselves to make filing bugs easier, more convenient, and less time consuming than it currently is today. There has got to be a way to do this and they would then get the feedback that they seem to be asking for.
It might also be nice to see how many duplicates there are in order to get an idea about how our reports are affecting the priority of an issue; sort of like Digg (if I correctly understand the meaning of the term 'digg').
A problem I often run into is code that runs the run loop recursively in the default mode. For example NSTask's isRunning property does this (on 10.5 at least, I haven't checked on 10.6 since I rolled my own NSTask), and of course this behavior has side-effects that you would expect: notifications being sent, timers firing, etc, all from within the -isRunning stack frame! Such side-effects can be fatal in some circumstances, if the code calling -isRunning is setting up state that the said notifications/timers/etc. rely on. I quickly learned to only run the run loop recursively in a custom mode; unfortunately Apple's frameworks don't obey this rule, and not once have I seen it documented when the frameworks do this.
-waitUntilExit. But you're right that this is absolutely an antipattern, and even worse with things like -isRunning where it doesn't even say that you do it.
Always run your sub-runloops in a custom mode! Definitely good advice to follow.
Let's say I have a class MyClass with a public class method myClassMethod. Now, it sounds like you are saying to call [self MyClass] every time I call [self myClassMethod], but I don't think I understand this. Calling [self myClassMethod] should be sufficient to utilize any override of myClassMethod written in a subclass, right?
[MyClass myClassMethod]; // bad, breaks subclasses
[[self class] myClassMethod]; // good
Comments RSS feed for this page
Add your thoughts, post a comment:
Spam and off-topic posts will be deleted without notice. Culprits may be publicly humiliated at my sole discretion.