8

After going through a beginner's iPhone developer book and reading sample code online, I've noticed that most Objective C programmers synthesize nearly every instance variable. Some variables are convenient to snythesize, but most should not when honoring the object oriented principle of encapsulation. The worst are synthetized properties marked as private. A C++ programmer trying to use someone else's code will read the public fields and methods in the header file. They will skip the private variables. This C++ programmer will not know that you intended the private properties to be used in some meaningful way.

Take a look at this sample template on lazy table image loading provided by Apple:

Header

@interface ParseOperation : NSOperation <NSXMLParserDelegate>

{
@private
    id <ParseOperationDelegate> delegate;
    NSData          *dataToParse;
    NSMutableArray  *workingArray;
    AppRecord       *workingEntry;
    NSMutableString *workingPropertyString;
    NSArray         *elementsToParse;
    BOOL            storingCharacterData;
}

Source

@interface ParseOperation ()
@property (nonatomic, assign) id <ParseOperationDelegate> delegate;
@property (nonatomic, retain) NSData *dataToParse;
@property (nonatomic, retain) NSMutableArray *workingArray;
@property (nonatomic, retain) AppRecord *workingEntry;
@property (nonatomic, retain) NSMutableString *workingPropertyString;
@property (nonatomic, retain) NSArray *elementsToParse;
@property (nonatomic, assign) BOOL storingCharacterData;
@end

@implementation ParseOperation
@synthesize delegate, dataToParse, workingArray, workingEntry, workingPropertyString, elementsToParse, storingCharacterData;

Now I know this is not C++ and we shouldn't assume all C++ practices should be honored in Objective C. But Objective C should have good reasons to stray away from general programming practices.

  1. Why are all the private ivars synthesized? When you look at the project as a whole, only NSMutableArray *workingArray is used by outside classes. So none of the other ivars should have setters and getters.
  2. Why are very sensitive ivars synthesized? For one, now that id delegate has a setter, the user of this object can switch the delegate in middle of the XML parsing, something that doesn't make sense. Also, NSData *dataToParse is raw XML data retrieved from the network. Now that it has a setter, the user of this object can corrupt the data.
  3. What's the point of marking everything private in the header? Since all ivars are are synthesized to have getters/setters, they are effectively public. You can set them to anything you want and you can get their value whenever you want.
JoJo
  • 18,399
  • 32
  • 103
  • 157
  • There were some questions similar to this yesterday: [Public read, private retain](http://stackoverflow.com/questions/5788458/public-read-private-retain-property) ; [Does a private @property create a private ivar?](http://stackoverflow.com/questions/5784875/does-a-private-property-create-an-private-instance-variable) I'm not sure if either of them answers your questions though. – jscs Apr 27 '11 at 17:10
  • You might also be interested in this: [iOS: must every iVar really be property?](http://stackoverflow.com/questions/5031230/ios-must-every-ivar-really-be-property) – Daniel Dickison Apr 27 '11 at 19:56

2 Answers2

10

I follow the idiom modeled by this example in many of my classes, so I can try to explain my own justification for this practice.

The properties in this example are declared in a class extension in the .m file. This makes them effectively private. Any attempt to access these properties from another class will cause a "Property not found" error upon compilation.

For developers coming from other languages, it may seem strange to synthesize getters and setters for private instance variables. Indeed, there is only one reason why I do this. When used consistently, synthesized properties can simplify memory management and help avoid careless mistakes that can lead to bugs. Here are a couple of examples:

Consider this:

self.workingPropertyString = [NSMutableString string];

versus this:

workingPropertyString = [[NSMutableString string] retain];

Many developers would claim that these two assignments are functionally equivalent, but there's an important difference. The second assignment leaks memory if workingPropertyString was already pointing at a retained object. To write code functionally equivalent to the synthesized setter, you'd have to do something like this:

NSMutableString *newString = [NSMutableString string];
if (workingPropertyString != newString) {
    [workingPropertyString release];
    workingPropertyString = [newString retain];
}

This code avoids leaking any existing object that the instance variable may be pointing to, and it safely handles the possibility that you may be re-assigning the same object to the instance variable. The synthesized setter does all of this for you.

Of course we can see that (workingPropertyString != newString) will always be true in this case, so we could simplify this particular assignment. In fact in most cases you can probably get away with a simple direct assignment to an instance variable, but of course it's the exceptional cases that tend to create the most bugs. I prefer to play it safe and set all my object instance variables through synthesized setters. All my instance object assignments are simple one-liners that look like this:

self.foo = [Foo fooWithTitle:@"The Foo"];

or this:

self.foo = [[[Foo alloc] initWithTitle:@"The Foo"] autorelease];

This simplicity and consistency gives my feeble brain less stuff to think about. As a result I almost never have bugs related to memory management. (I'm aware that the autorelease idiom could theoretically consume excessive memory in a tight loop, but I have yet to encounter that issue in practice. If I ever do, it's a simple case to optimize.)

One other thing I like about this practice is that my dealloc methods all look like this:

- (void)dealloc {
    self.delegate = nil;
    self.dataToParse = nil;
    self.workingArray = nil;
    self.workingEntry = nil;
    self.workingPropertyString = nil;
    self.elementsToParse = nil;
    [super dealloc];
}

EDIT: Daniel Dickison pointed out some risks to using accessors in dealloc that I hadn't considered. See the comments.

where every object property is simply set to nil. This simultaneously releases each retained property while setting it to nil to avoid certain crashes due to EXC_BAD_ACCESS.

Note that I've set self.delegate = nil; even though that property was declared as (nonatomic, assign). This assignment wasn't strictly necessary. In fact, I could do away with properties for my (nonatomic, assign) objects altogether, but again I've found that applying this idiom consistently across all my instance variables gives my brain less to think about, and further reduces the chance that I'll create a bug through some careless mistake. If necessary I can simply flip a property from (nonatomic, assign) to (nonatomic, retain) without having to touch any memory management code. I like that.

One could also use consistency as an argument for synthesizing properties for private scalar variables, as your example has done in the case of BOOL storingCharacterData;. This practice ensures that every instance variable assignment will look like self.foo = bar;. I don't usually bother to create private scalar properties myself, but I can see some justification for this practice.

cduhn
  • 17,498
  • 4
  • 47
  • 64
  • 5
    Nice answer, but one nit-pick: setting properties to nil rather than calling `release` directly in the `dealloc` method is discouraged (there's some Apple docs saying that, though I can't locate it right now). The reason being that the setter can fire a KVO notification which could lead to observers trying to access a partially deallocated instance. – Daniel Dickison Apr 27 '11 at 19:54
  • Ewww. Good point. I haven't done much with KVO, so I haven't encountered that case, but it makes sense. I'll have to seek out those Apple docs and possibly reconsider how I write my deallocs. Thanks for opening my eyes to this risk. – cduhn Apr 27 '11 at 20:06
  • Apple's documentation seems to be in an inconsistent state on this point. The Objective C Programming Language documentation says that if you're using synthesized instance variables with the modern runtime, "you must invoke the accessor method" since you can't access the instance variable directly. However, it appears that in the latest tools you can access the ivar directly, so I guess the idiom I'll use in dealloc is `[ivar release], ivar = nil;`. There's a good discussion about this here: http://stackoverflow.com/questions/1283419/valid-use-of-accessors-in-init-and-dealloc-methods – cduhn Apr 27 '11 at 20:54
  • 1
    Another good discussion on this topic: http://www.mikeash.com/pyblog/friday-qa-2009-11-27-using-accessors-in-init-and-dealloc.html – Daniel Dickison Apr 27 '11 at 21:08
  • Great stuff Daniel. Thanks for the link. I remain on the fence. – cduhn Apr 27 '11 at 21:26
0

Why are all the private ivars synthesized? When you look at the project as a whole, only NSMutableArray *workingArray is used by outside classes. So none of the other ivars should have setters and getters.

No real need; if you are going to access all the ivars directly anyway, there is no need for @synthesize.

Why are very sensitive ivars synthesized? For one, now that id delegate has a setter, the user of this object can switch the delegate in middle of the XML parsing, something that doesn't make sense. Also, NSData *dataToParse is raw XML data retrieved from the network. Now that it has a setter, the user of this object can corrupt the data.

None of the setter/getters are publicly declared. If a client of the class wanted to corrupt things by switching the delegate in the middle, they'd have to break encapsulation to do so.

So, ultimately, a non-issue.

What's the point of marking everything private in the header? Since all ivars are are synthesized to have getters/setters, they are effectively public. You can set them to anything you want and you can get their value whenever you want.

Note that there is no need to even declare the ivars in that example; the compiler will automatically synthesize them based on the @property declaration.

Traditionally, @private protected against someone diddling the ivar directly from externally to an instance of the class.

Note that anInstance->ivar or self->ivar is almost never used (and, when used, it is almost always for the wrong reason). There are uses for it, but it is rare.

bbum
  • 160,467
  • 23
  • 266
  • 355
  • Seems like Objective C should *enforce* encapsulation rather than *suggest* encapsulation by the way the code is laid out. After reading through the entire project, I can see that the Apple programmer *suggests* me to not tamper with the private ivars, but I shouldn't have to do that. I should be able to just read the header file to understand how to use this class and have the compiler *enforce* the encapsulation. – JoJo Apr 27 '11 at 18:10
  • What's wrong with `anInstance->publicIVar` and `self->trulyPrivateIvar` if that enforces encapsulation? – JoJo Apr 27 '11 at 18:13
  • `anInstance->publicIvar` isn't a pattern that is used; it bypasses any accessor method(s) and, thus, means that subclasses cannot override behavior on access or set (it also means that KVO won't work). `self->ivary` is noise; unnecessary. – bbum Apr 27 '11 at 19:27
  • 1
    Encapsulation is effectively enforced in this example because the properties are declared in a class extension within the .m file rather than in the .h file. If an object of another class attempts to access the properties using dot-syntax it will yield an error. An attempt to call the setter or getter directly will yield a warning. An external class would have to go out of its way to break this encapsulation by doing something deliberate like `[operation performSelector:@selector(setDelegate:) withObject:newDelegate]`. – cduhn Apr 27 '11 at 19:37