Where to declare variables of a method that's called many times

Question

I have the following method that is called in my tableView's cellForRowAtIndexPath: method.

- (void)animateTheEqualizer
{
    UIImage *frame1 = [UIImage imageNamed:@"equalizer1"];
    UIImage *frame2 = [UIImage imageNamed:@"equalizer2"];
    UIImage *frame3 = [UIImage imageNamed:@"equalizer3"];
    UIImage *frame4 = [UIImage imageNamed:@"equalizer4"];
    UIImage *frame5 = [UIImage imageNamed:@"equalizer5"];
    UIImage *frame6 = [UIImage imageNamed:@"equalizer6"];
    UIImage *frame7 = [UIImage imageNamed:@"equalizer7"];
    UIImage *frame8 = [UIImage imageNamed:@"equalizer8"];
    UIImage *frame9 = [UIImage imageNamed:@"equalizer9"];
    UIImage *frame10 = [UIImage imageNamed:@"equalizer10"];

//  UIImageView *equalizer;

    equalizer.animationImages = [[NSArray alloc] initWithObjects:frame1, frame2, frame3, frame4, frame5, frame6, frame7, frame8, frame9, frame10, nil];
    [equalizer startAnimating];
}

cellForRowAtIndexPath: method:

{   
    ...
    equalizer = (UIImageView *) [cell viewWithTag:30];
    equalizer.hidden = YES;
    if (entry.audioManager && [entry.audioManager soundPlayer].isPlaying)
    {
        equalizer.hidden = NO;
        [self animateTheEqualizer];
    }
    return cell;
}

I had to declare equalizer in the header, because I access it in both methods. However for all the UIImages named frame1, frame2, ..., frame10 I'm not sure where is the best place to be declared, since they're only used in that method, but declaring them every time the method is called in cellForRowAtIndexPath: seems not to be memory efficient, while I'm not sure if declaring them in the header publicly, is a better choice.

Which is better and more efficient?

---

How I ended up really solving this:

Thanks to all those who answered, I used a combination of @Antonio and @x4h1d 's answers as follows:

@implementation MyClass
static NSMutableArray *imageArray;
static UIImage *frame;

And then + (void) initialize method:

+ (void) initialize
{
    imageArray = [[NSMutableArray alloc] init];
    for(NSUInteger i = 1; i <= 10; i++)
    {
        UIImage *frame = [UIImage imageNamed:[NSString stringWithFormat:@"equalizer%lu",(unsigned long)i]];
        if(frame)
        {
            [imageArray addObject:frame];
        }
        else
        {
            // handle if image is not there
        }
        frame = nil;
    }
}

and then this bit in my cellForRowAtIndexPath: method for the tableView:

equalizer = (UIImageView *) [cell viewWithTag:30];
equalizer.hidden = YES;
if (entry.audioManager && [entry.audioManager soundPlayer].isPlaying)
{
    equalizer.hidden = NO;
    [self animateTheEqualizer];
}

Answer 1

delcare a iVar or private NSMutableArray, say imageArray. In ViewDidLoad method and insert images as follow,

imageArray = [[NSMutableArray alloc] init];
for(NSUInteger i = 1; i <= 10; i++)
 {
  UIImage *frame = [UIImage imageNamed:[NSString stringWithFormat:@"equalizer%i",i]];
  if(frame){
    [imageArray addObject:frame];
  }else {
    // handle if image is not there
  }
  frame = nil;
}

equalizer.animationImages = [imageArray copy];
[equalizer startAnimating];

if images are static and not used outside the class, I think this will be the efficient way . btw, set imageArray to nil if you think you don't need it anymore.

Answer 2

You should declare equaliser in .h file just if you want to expose this property in other classes. If you need it just in this class declare private one in .m file.

You don't have to declare 10 UIImages (properties) if you haven't got good reason to do that. In your example you create 10 local UIImage variable in animateTheEqualizer method and it works fine. If you need access it in any other place you can call:

UIImage *frame1 = equalizer.animationImages[0];

EXTENDED:

You can load the array just one times, let's say in viewDidLoad:

   UIImage *frame1 = [UIImage imageNamed:@"equalizer1"];
    UIImage *frame2 = [UIImage imageNamed:@"equalizer2"];
    UIImage *frame3 = [UIImage imageNamed:@"equalizer3"];
    UIImage *frame4 = [UIImage imageNamed:@"equalizer4"];
    UIImage *frame5 = [UIImage imageNamed:@"equalizer5"];
    UIImage *frame6 = [UIImage imageNamed:@"equalizer6"];
    UIImage *frame7 = [UIImage imageNamed:@"equalizer7"];
    UIImage *frame8 = [UIImage imageNamed:@"equalizer8"];
    UIImage *frame9 = [UIImage imageNamed:@"equalizer9"];
    UIImage *frame10 = [UIImage imageNamed:@"equalizer10"];

//  UIImageView *equalizer;

    equalizer.animationImages = [[NSArray alloc] initWithObjects:frame1, frame2, frame3, frame4, frame5, frame6, frame7, frame8, frame9, frame10, nil];

You use just local variable for UIImage.

After that when you want to call animation you call:

[equalizer startAnimating];

Answer 3

If all instances of the table view can share the same frame variables then you can alter your existing declarations as shown below. The static declaration will only be executed once in total and the variables will then be available - in that method only - whenever the method is subsequently called, including by any other instances of the table view.

- (void)animateTheEqualizer
{
    static UIImage *frame1 = [UIImage imageNamed:@"equalizer1"];
    static UIImage *frame2 = [UIImage imageNamed:@"equalizer2"];
    static UIImage *frame3 = [UIImage imageNamed:@"equalizer3"];
    static UIImage *frame4 = [UIImage imageNamed:@"equalizer4"];
    static UIImage *frame5 = [UIImage imageNamed:@"equalizer5"];
    static UIImage *frame6 = [UIImage imageNamed:@"equalizer6"];
    static UIImage *frame7 = [UIImage imageNamed:@"equalizer7"];
    static UIImage *frame8 = [UIImage imageNamed:@"equalizer8"];
    static UIImage *frame9 = [UIImage imageNamed:@"equalizer9"];
    static UIImage *frame10 = [UIImage imageNamed:@"equalizer10"];

//  UIImageView *equalizer;

    equalizer.animationImages = [[NSArray alloc] initWithObjects:frame1, frame2, frame3, frame4, frame5, frame6, frame7, frame8, frame9, frame10, nil];
    [equalizer startAnimating];
}

Answer 4

If images don't change, the best approach is to declare them as static

@implementation MyClass

static UIImage *frame1, *frame2, *frame3, *frame4, *frame5, *frame6, *frame7, *frame8, *frame9, *frame10;

+ (void) initialize {
    frame1 = [UIImage imageNamed:@"equalizer1"];
    frame2 = [UIImage imageNamed:@"equalizer2"];
    frame3 = [UIImage imageNamed:@"equalizer3"];
    frame4 = [UIImage imageNamed:@"equalizer4"];
    frame5 = [UIImage imageNamed:@"equalizer5"];
    frame6 = [UIImage imageNamed:@"equalizer6"];
    frame7 = [UIImage imageNamed:@"equalizer7"];
    frame8 = [UIImage imageNamed:@"equalizer8"];
    frame9 = [UIImage imageNamed:@"equalizer9"];
    frame10 = [UIImage imageNamed:@"equalizer10"];    
}

the initialize method is a static constructor, and it is called the first time the class is used. The benefit in using static variables is that they are instantiated once, with inherent advantages about speed (operation performed once) and memory (images instantiated once).

Another approach is instantiating the array statically:

@implementation MyClass

static NSArray *_animationImages;

+ (void) initialize {
    _animationImages = @[
        [UIImage imageNamed:@"equalizer1"],
        [UIImage imageNamed:@"equalizer2"],
        [UIImage imageNamed:@"equalizer3"],
        [UIImage imageNamed:@"equalizer4"],
        [UIImage imageNamed:@"equalizer5"],
        [UIImage imageNamed:@"equalizer6"],
        [UIImage imageNamed:@"equalizer7"],
        [UIImage imageNamed:@"equalizer8"],
        [UIImage imageNamed:@"equalizer9"],
        [UIImage imageNamed:@"equalizer10"] 
    ];
}

and then in your method:

- (void)animateTheEqualizer {
    equalizer.animationImages = _animationImages;
    [equalizer startAnimating];
}

The latter solution is better in my opinion, but of course it's usable if _animationImages doesn't need to be updated (such as changing images or their order).

Answer 5

Place them here as private variables:

@implementation viewController
{
   UIImage *frame1;
   ...
}

Then place all of these in your viewDidLoad:

frame1 = [UIImage imageNamed:@"equalizer1"];

you shouldn't be declaring them or init them every time that function is called if they don't change