John Lee
John Lee

Reputation: 1251

Objective-C : Need advice on setting instance variables in init method

I am using ARC.

This is my .h file

...
- (id)initWithCoordinate:(CLLocationCoordinate2D)c title:(NSString *)t;

@property (nonatomic, readonly) CLLocationCoordinate2D coordinate;
@property (nonatomic, copy) NSString *title;
...

This is my .m file

....
@synthesize coordinate, title;

- (id)initWithCoordinate:(CLLocationCoordinate2D)c title:(NSString *)t
{
    self = [super init];
    if (self) {
        coordinate = c;
        [self setTitle:t];
     }
    return self;
}
....
  1. Is setting coordinate this way, the right way to do it? Given that I declare it as readonly, it seems like it is the only way to do it. What if I just use the default (i.e. readwrite), in this case, should I use the setter method [self setCoordinate] instead?

  2. I could set the title by doing title = t as well. Compare to using the setter method, the result is the same, but what is the difference ?


Thanks! Wish I could accept all of your answers.

Upvotes: 1

Views: 2009

Answers (5)

Sean
Sean

Reputation: 5820

You're actually supposed to set ivars directly in an initializer method all the time. This is true whether or not you have a readonly or readwrite property. The documentation here even says so.

The reasoning behind this has to do with inheritance. If someone were to subclass your class and overwrite the setters for your properties such that they bypass the ivars you created (or do some other wacky thing), then suddenly your original implementation of your initializer method now no longer does what it is written to do. In particular, your initializer could end up creating an object with a weird state due to the subclass overriding your accessors. In the pre-ARC days, you could also end up with tricky (or just straight-up broken) memory situations when this sort of thing happens. The take-away message is: you should write initializers so that they will always create an object with a known valid state.

So (assuming you're using ARC) your initializer should actually be:

- (id)initWithCoordinate:(CLLocationCoordinate2D)c title:(NSString *)t
{
    self = [super init];
    if (self) {
        coordinate = c;
        title = [t copy];
     }
    return self;
}

Personally, I prefer to synthesize ivars with a starting underscore to clarify when I'm using the property and when I'm accessing the ivar directly (LLVM 4.0 now does this to automatically synthesized properties as well).

@synthesize coordinate = _coordinate;
@synthesize title = _title;

- (id)initWithCoordinate:(CLLocationCoordinate2D)c title:(NSString *)t
{
    self = [super init];
    if (self) {
        _coordinate = c;
        _title = [t copy];
     }
    return self;
}

Upvotes: 3

Metabble
Metabble

Reputation: 11841

1: As your code is now, yes, that is the right way to do it. If you weren't using ARC (assuming you are currently), you'd also want to retain the value to assert ownership. This will be done automatically under ARC. Keep in mind that that is not the only way of doing it; you could redeclare the property as readwrite in the class extension in the implementation file. This is a common practice which allows you to have the benefits of a readwrite property while having the property still be readonly to users of the class. Ex.

//MyClass.h

@interface MyClass : NSObject
@property (nonatomic, strong, readonly) NSNumber* number;
- (void) initWithNumber:(NSNumber*)number;
@end

//MyClass.m

@interface MyClass ()
@property (nonatomic, strong, readwrite) NSNumber* number;
@end

@implementation MyClass
//this changes the instance variable backing the property to _number.
@synthesize number = _number;

- (void) initWithNumber:(NSNumber*)number{
    self = [super init];
    if (self) {
        self.number = number;
    }
    return self;
}
@end

At the end of the day, I'd say it's a good habit to use setters whenever you can to keep things KVO compliant and so that you always know when values change. For instance, if you have a custom UIView with a property that is reflected in its appearance, chances are you'd want to redisplay yourself when it changes. The easiest way to do this is to implement the setter yourself and call setNeedsDisplay after setting the value. You couldn't do that if you set the instance value backing the property directly; the user of the class would have to remember to call setneedsDisplay every time they set it, manually.

2: One goes through the setter method, giving you a way to know when a value is going to be set, while one sets a value to the instance variable backing the property. The setter method will always handle memory management in the way it was told to, while it's up to you to do things such as copying values for a copy setter if you assign directly to an instance variable, so that you maintain some consistent scheme. Going through setters sometimes, and not others can lead to some nasty bugs if you don't be careful. Never going through setters makes it hard to know when values change, making it near impossible to weed out invalid values. For instance, if you had an int property you wanted to limit to values in some range and someone passed in a value under the minimum limit, you'd probably want to set the property to the lowest possible value in the range. You can't do that without the value going through the setter first.

Upvotes: 2

rob mayoff
rob mayoff

Reputation: 385920

  1. Setting the coordinate that way is correct, and is the only way to do it if you have declared the property readonly.

  2. Setting the title using title = t is different than setting the title using [self setTitle:t]. If you directly assign to the instance variable, you will just retain the NSString instance that was passed as argument t. But if you using the accessor method, the accessor will ask the string to copy itself (because you declared the property copy). If the string you were given as argument t is actually an NSMutableString, then you will get an immutable copy of it. If the string you were given as argument t is already an immutable string, it will just return itself when asked for a copy.

Upvotes: 1

Paul.s
Paul.s

Reputation: 38728

self.coordinate = c;

is essentially compiled to be the same as calling

[self setCoordinate:c];

The difference between coordinate = c and [self setCoordinate:c]; is that the first is just setting a variable directly where as the second is calling a method.

The reason to be wary is that methods could potentially have side effects depending on how the implementation is written e.g. (stupid example)

- (void)setCoordinate:(CLLocationCoordinate2D)coordinate;
{
    _coordinate = coordinate;
    [self doSomethingCrazy];
}

Upvotes: 0

Stig Brautaset
Stig Brautaset

Reputation: 2632

Yes, it is fine to set it like that. If you prefer to use a property all the time you can override the property to be read/write rather than read-only in a class extension. In Foo.m:

@interface Foo ()
@property (nonatomic) CLLocationCoordinate2D coordinate;
@end

@implementation Foo {
    // ...
    self.coordinate = c;
}

Upvotes: 1

Related Questions