1

If I have a property, say an NSArray, that's going to be initialized only once for each instance of my class, is there anything wrong with this:

(in the interface)

@property(strong, nonatomic)NSArray *bubbleArr;

(in the implementation)

-(NSArray*)bubbleArr
{
    if(!bubbleArr)
    {
        NSMutableArray *tempBubbArr = [[NSMutableArray alloc] init];
        // get filepath for first speech bubble image for page
        NSString *speechBubbleImgPath = [[NSBundle mainBundle] pathForResource:
                                         [NSString stringWithFormat:@"speech_%i_0", pageIndex]
                                                                        ofType:@"png"];

        for(int i = 1; speechBubbleImgPath; i++)
        {
            UIImage *speechBubbleImg = [[UIImage alloc] initWithContentsOfFile:speechBubbleImgPath];
            UIImageView *speechBubbleImgView = [[UIImageView alloc] initWithImage:speechBubbleImg];

            [tempBubbArr addObject:speechBubbleImgView];

            speechBubbleImg = nil;
            speechBubbleImgView = nil;
            speechBubbleImgPath = nil;

            speechBubbleImgPath = [[NSBundle mainBundle] pathForResource:
                                   [NSString stringWithFormat:@"speech_%i_%i", pageIndex, i]
                                                                  ofType:@"png"];
        }

        bubbleArr = [[NSArray alloc] initWithArray:tempBubbArr];

        tempBubbArr = nil;
    }

    return bubbleArr;
}

I've never used custom accessor methods, but this seems like a clean way to set it up, so I don't have to set up each property in my viewDidLoad or elsewhere, and don't have to worry about it being nil. I don't recall ever actually come across code that does this. Is this the recommended way to do it? Also, I'll always want to use self.bubbleArr to make sure this method is called, right?

Marty
  • 5,926
  • 9
  • 53
  • 91
  • 1
    side note: be aware that if you're not using ARC, your code above is leaking – Sean May 28 '12 at 22:30
  • 2
    Looking at the code, I'm guessing you don't want the array reference to be changed since it is a one-time setup. You might consider making it a `readonly` property to ensure that. Otherwise code like `object. bubbleArr = [[NSArray alloc] init]`; could screw up things. – Anurag May 28 '12 at 22:40
  • 2
    Absolutely nothing wrong with using a lazy initialization technique inside a getter. However, for a non-readonly property you need to be sure that the setter is appropriately aware of how things work, and if async access (multithreading) is a possibility you need to take that into account in both getter and setter. (I've not attempted to analyze/critique your implementation.) – Hot Licks May 28 '12 at 22:46
  • Ah ok I was wondering if I should have it be readonly. Thanks! – Marty May 29 '12 at 00:49

4 Answers4

2

This is a totally valid way of setting up your property. Apple does this sort of thing very often in their sample code as well as in their project templates. Look, for example, at the core data stack setup in a newly created core data iOS project. As @WendiKidd noted, you have to access your variable through the accessors all the time to make sure this works nicely (which is probably what you should be doing anyway).

In particular, this is a good way of implementing a property of a class that can really only return one thing (which, from your comment, it sounds like is what you're trying to do). If that's your goal, here are some guidelines to follow:

  1. Declare your property as readonly
  2. Declare it in the public header if it should be publicly accessible, or in a class extension in the .m file if it should be "private"
  3. If it can/should be backed by a variable, synthesize the ivar and overwrite the getter as you've done
  4. If it shouldn't/doesn't have to be backed by a variable, declare the property as @dynamic in the implementation and overwrite the getter
  5. Only access your ivar through the accessor

Declaring the variable as dynamic in the 4th point will signal to anyone that looks at your code that you've likely written a custom accessor for that property.

Sean
  • 5,810
  • 2
  • 33
  • 41
  • @bbum -- Very true. I do it as a reminder to myself that I have overridden the accessors without synthesizing an ivar. I suppose that's really a matter of personal taste... – Sean May 28 '12 at 23:09
  • I don't think point 1 is all that valid, as it's often handy to have a default object loaded when one has not been specifically configured and assigned. – Paul.s May 28 '12 at 23:37
  • @Paul.s -- I agree, but the OP wrote in the comments to WendiKidd's answer that "I was pretty much going for a 'constant array' effect since it can/should only be one specific array for each instance" which is what this answer addresses and what I explained in the paragraph preceding my points – Sean May 28 '12 at 23:43
  • on a side note...using ARC, i usually set my properties to `nil` in `viewDidUnload`. However, `readonly` doesn't let me do that. how do you make sure the memory is freed? – Marty May 29 '12 at 00:57
  • Using ARC, the memory will be freed in the `-dealloc` method where the instance variable will be sent a `-release` message directly. ARC handles this all for you, so you don't need to implement `-dealloc` yourself – Sean May 29 '12 at 01:10
  • @sean a viewController can live long after `viewDidUnload`. What you can do is publicly declare the property as `@property (strong, nonatomic, readonly) NSArray *bubbleArr;` and then in a class extension make it read/write to your implemtation by redeclaring it as `@property (strong, nonatomic) NSArray *bubbleArr;` – Paul.s May 29 '12 at 09:52
  • @Paul.s -- Yup, you can do that as well. But if your goal is to truly have the property be `readonly` (perhaps even from the perspective of the .m file), then you may not want to do this since it will synthesize a setter which could be used to write over the object allocated in the getter. If the goal is to have a strictly `readonly` property, one could also explicitly deallocate the ivar and set it to `nil` in `-viewDidUnload` if it is necessary to clean up this particular variable in `viewDidUnload` – Sean May 29 '12 at 14:10
1

The technique of waiting until you need the contents of a variable to initialize it is called 'lazy loading', and is a valid technique. I'm not sure about doing it inside the getter, though.

I think the problem is described exactly by the last line of your post--yes, you would always have to make sure you called the getter method when you wanted to reference the object, even inside the class itself. It's easy to make mistakes and not do that properly, and it's an especially bad idea if your code could be passed on to another developer in the future. They are absolutely not going to expect that you've set things up this way, and could end up with problems when they expect to be able to access the variable as normal. It is common and accepted practice to initialize member variables inside viewDidLoad.

So yes--this is technically possible, though not a very sound setup, design-wise. I would recommend strongly against it. But if you simply want to know if it will function, the answer is yes.

WendiKidd
  • 4,333
  • 4
  • 33
  • 50
  • I don't know that they are guaranteed to be init to nil either, but I've never seen a case where it does not happen. – Andy Obusek May 28 '12 at 22:24
  • 2
    http://stackoverflow.com/questions/990817 - further, this is pretty common and any future maintainer should be aware of the idiom. It's also common to name the instance variable `_bubbleArr` as a defense against accidentally referencing the instance variable directly. – Ian Henry May 28 '12 at 22:26
  • @IanHenry And there's that curiosity answered! Thanks! Very good to know :) – WendiKidd May 28 '12 at 22:28
  • 3
    I would argue this is a valid technique (I'm not sure about this specific example as I've not studied it for side effects etc). Dumping this code in `viewDidLoad` does not really make that much sense as you may not be using it when the view loads. It is normally always a good idea to go through properties anyway to get good encapsulation and more flexible easy to change code. – Paul.s May 28 '12 at 22:28
  • @JacquesCousteau Ah, perhaps my terminology was confusing--I'll make an edit to change it. When I mentioned the code being inherited by someone else, I didn't mean actual class inheritance--I meant the code being passed on to and maintained by another developer :) And all right--I can see the points you guys are making. I still wouldn't write the code this way myself, because I feel it would be easy to make a mistake, but I understand why it is a valid method for others to use in their code. – WendiKidd May 28 '12 at 22:33
  • 1
    This is actually a pretty standard design in objective-c, so if your code is being given to someone else who has spent some time writing the language, they should find this to be a perfectly normal way of doing things. – Sean May 28 '12 at 22:36
  • @Sean Hmm. Personally I've never seen lazy loading in getter methods, though I agree it's quite prominent elsewhere (and of course believe you if you've seen it in getters!). (And I do have a good bit of experience with Objective-C). But that's all right, I hear and understand everyone's points of view and can agree to disagree :) – WendiKidd May 28 '12 at 22:39
  • @WendiKidd all ivars are guaranteed to be zero'd at by the time the `+alloc` method returns. Thus for properties that are backed by ivars, they are guaranteed to be `0`/`nil`/etc. – Dave DeLong May 28 '12 at 22:41
  • @WendiKidd -- check out the AppDelegate in a newly created iOS project in Xcode 4.3 with Core Data enabled. You will see that the getters for `-managedObjectModel`, -`persistentStoreCoordinator`, and `-managedObjectContext` are all set up in exactly this way – Sean May 28 '12 at 22:41
  • If the instance variable is declared in the header file, then there is the possibility of subclasses to access it directly, hence screwing up the encapsulation. But there's rarely a need to define the instance variable, as that is redundant and harder to maintain. – Anurag May 28 '12 at 22:43
  • Great, very informative points made. I was pretty much going for a "constant array" effect since it can/should only be one specific array for each instance. Thanks! – Marty May 28 '12 at 22:44
0

Yes, if you don't use self.bubbleArray OR [self bubbleArray] you will not be invoking that method.

Other than that, totally a fine way of managing a property's instantiation.

Andy Obusek
  • 12,614
  • 4
  • 41
  • 62
0

It seems like a clean solution, it feels a bit like lazy loading. But I am not sure I would do it like this.

Wim Haanstra
  • 5,918
  • 5
  • 41
  • 57