1

Today I was at interview and was asked a question:

Generate setter and getter by hands for proper declaration using manual reference counting:

@interface SomeClass : NSObject 
{
     NSMutableArray* _array; 
}
@property (copy) NSArray* array;

@end

My answer was:

- (NSArray *)array 
{
     @syncronized (self)
     {
         return [_array copy];
     }  
}

- (void)setArray:(NSArray *)array 
{    
    @synchronized (self)    
    {    
      if (_array != array)
      {
         [_array release];
         _array = [array mutableCopy];
         [_array retain]
      }    
    } 
}

I never worked using MRC so not sure about correctness of an answer. Please help me to correct this code with description!

Olex
  • 1,656
  • 3
  • 20
  • 38
  • 1
    possible duplicate of [Explicit getters/setters for @properties (MRC)](http://stackoverflow.com/questions/21802069/explicit-getters-setters-for-properties-mrc) – jscs Feb 15 '14 at 22:01
  • 2
    Thanks Josh, but I am asking about mutable/immutable ivar/property. So my question is about what do I need to set and get in this particular case – Olex Feb 15 '14 at 22:03
  • 1
    Also I don't know is there any sense or it is just the case for abstract test – Olex Feb 15 '14 at 22:08
  • 3
    It's been a long time since I used MRC, but I think you're leaking the copy in the getter (should be `return [[_array copy] autorelease]`) – Aaron Brager Feb 15 '14 at 22:12
  • Thanks @AaronBrager ! Question is also about different types of ivar and property. Can you comment this? Is it normal and really can be used in the daily practice? – Olex Feb 15 '14 at 22:20
  • 1
    It can be done. [How to declare an immutable property backed by a mutable type?](http://stackoverflow.com/q/7710400) – jscs Feb 15 '14 at 22:21
  • @JoshCaswell All the examples in that answer *declare* immutability, but *actually* return the mutable instance variable. – Aaron Brager Feb 15 '14 at 22:23
  • 2
    @Alex It's normal to override getters and setters, so you might see code like this in a real app. But there's no good reason to use MRC, so just transition the project to ARC after they hire you :) – Aaron Brager Feb 15 '14 at 22:24
  • 1
    Aaron, so is everything fine with return types and assigns, especially inside getter? – Olex Feb 15 '14 at 22:31
  • Copying is not always required, @AaronBrager. I believe modifying an object which was declared as immutable (return type of getter) because you know it's actually mutable should be considered an error. See http://stackoverflow.com/a/3922549/ for some reasoning. – jscs Feb 15 '14 at 22:32
  • @JoshCaswell I guess that's fair. In classes like this, I would typically maintain two instance variables (one mutable, one immutable), and recreate the immutable one when the mutable one changes. – Aaron Brager Feb 16 '14 at 00:01
  • (This allows me to mutate the contents of my internal variable on a different thread without worrying that some other class will modify it.) – Aaron Brager Feb 16 '14 at 00:03
  • @JoshCaswell, would you mind making a revision of the answer I've just posted? – Stanislav Pankevich Feb 16 '14 at 12:00
  • @AaronBrager, would you mind making a revision of the answer I've just posted? – Stanislav Pankevich Feb 16 '14 at 12:01

1 Answers1

5

I am the author of one of the linked topics and I think now I understand MRC enough to write this answer here:

1) You're obviously leaking the copy in the getter (see it also in the comments) - so it should be balanced by corresponding autorelease call.

Also note, that this copy inside your getter is done because of you need to return immutable object, not because of getters for @properties declared with (copy) require you to do so!

2) Your setter should not retain after mutableCopy, since mutableCopy already does +1 for you.

See the following quotes from Advanced Memory Management Programming Guide

Basic Memory Management Rules.

You own any object you create

You create an object using a method whose name begins with “alloc”, “new”, “copy”, or “mutableCopy” (for example, alloc, newObject, or mutableCopy).

And

Ownership Policy Is Implemented Using Retain Counts

The ownership policy is implemented through reference counting—typically called “retain count” after the retain method. Each object has a retain count.

When you create an object, it has a retain count of 1.

3) In my topic's comments @robmayoff shared the link to open source implementation of runtime: reallySetProperty in objc-accessors.mm with the following reasoning behind it:

The nonatomic retain and copy setters unfortunately have an unnecessary race condition. If, on thread 1, the setter releases _count, and on thread 2 the getter accesses _count before thread 1 has set _count = [count retain], thread 2 may access a deallocated object. Always store the new value in _count before releasing the old value. The real accessor in the Objective-C runtime does it correctly. See reallySetProperty in objc-accessors.mm. – rob mayoff

4) You example is also missing dealloc since you were to write it under MRC.

5) [IMO, maybe subjective] Since your setter is creating copies of array argument, you don't need to have this if (_array != array) check since the task of (copy) setter is, I believe, to produce copies of what is passed, so I think this is may be omitted.

Having these points in mind I would write your example like the following:

- (NSArray *)array 
{
     id array;
     @synchronized (self)
     {
         array = [_array copy];
     }
     return [array autorelease];  
} 

- (void)setArray:(NSArray *)array 
{    
    id oldValue;
    @synchronized (self)    
    {    
        oldValue = _array;
        _array = [array mutableCopy];
    } 
    [oldValue release];
}

- (void)dealloc {
    [_array release];
    [super dealloc];
}

In answer to your question in the comments:

Is it normal and really can be used in the daily practice?

I would say, that it can be used in a daily practice with the following additional considerations:

1) You should move you ivar declaration into a private category @interface SomeClass () be it inside your .m file or a private class extension.

2) You should make your getters/setters nonatomic since atomicity of this property is on your shoulders (you already do synchronized on your own in both setter and getter).

3) See also the setup from linked topic which omits ivar and uses second @property declaration. In your case it would look like this:

// .h
@interface SomeClass : NSObject
@property (nonatomic, strong, readonly) NSArray *array;
@end

// .m or private class extension
@interface SomeClass()
@property (nonatomic, strong) NSMutableArray *array;
@end

@implementation SomeClass
// and here your getters/setters
@end

This setup looks promising though I haven't really tested it for the case like yours.


P.S. Recently I did some research for this back-to-the-past Manual Reference Counting, let me share with you the following links which I found to be the best on this topic:

Community
  • 1
  • 1
Stanislav Pankevich
  • 11,044
  • 8
  • 69
  • 129