views:

54

answers:

4

Here is some of the code from one of my classes, I was wondering am I handling the memory right or am I leaking anywhere?

@implementation CardViewController
@synthesize playerImage;
@synthesize cardLabel;
@synthesize card;
@synthesize frontView;
@synthesize backView;
@synthesize nameLabel;
@synthesize infoLabel;
@synthesize delegate;

-(void) initialiseButtons 
{
NSLog(@"initialiseButtons  %d",totalButtons);
int ypos = playerImage.frame.origin.y + playerImage.frame.size.height + 42;
for(int i=0; i<totalButtons; i++) 
{       
    StatView *sv = [[StatView alloc] initWithYPos:ypos];
    sv.tag = 100 + i;
    [sv.overlayButton addTarget:self action:@selector(statTapped:)    
                 forControlEvents:UIControlEventTouchUpInside];
    sv.overlayButton.tag = 10 + i;
    [self.frontView addSubview:sv];
    ypos += 26;
}
}

-(IBAction) statTapped:(id) sender 
{

UIButton *tappedButton = (UIButton *)sender;
int tag = tappedButton.tag;
if(delegate && [delegate respondsToSelector:@selector(cardTapped:)]) {
    [delegate cardTapped:tag-10];
}
 }

-(void) viewDidLoad 
{
NSLog(@" viewDidLoad CVC");
[self initialiseButtons];

metaData = [[NSArray alloc]     
      initWithObjects:@"Height",@"Weight",@"Games",@"Attack",@"Defense", nil];
 }

-(void) setCard:(Card *)newCard 
{
NSLog(@"setCard");
[card release];
card = [newCard retain];
[self updateUI];
}

- (void)dealloc 
{
[card autorelease];
[metaData autorelease];
 [super dealloc];
}

@end

Where should I release StatView *sv = [[StatView alloc] initWithYPos:ypos]; If i released it every loop wouldnt that cause problems? Besides that do I handle the rest of the memory ok?

Thanks -Code

+1  A: 

Yes, you should release that StatView at the end of each loop iteration, when you've inserted it into the view hierarchy.

Chuck
A: 

You should release it as soon as you've added it to the view hierarchy (by sending addSubview: with the newly-allocated view as an argument). This is because UIView objects retain their subviews.

One other problem I notice: your setCard method should first check whether the new and old cards are identical, and do nothing in that case. Otherwise, you may release your existing card, then try to retain it again, only to find that it has been dealloced.

David M.
+1  A: 

You should try the analyzer built into XCode as it is very good at finding these type of memory leaks. Have a look.

willcodejavaforfood
+1  A: 
  1. Release the new StatView after this line

    [self.frontView addSubview:sv];

    [sv release]; // frontView retains sv

  2. Release all properties declared as retain or copy in dealloc. Candidate properties: playerImage, cardLabel etc. Send a release message, not autorelease

    \\[card autorelease];

    [card release];

  3. In viewDidUnload release all properties that are declared as IBOutlet and set the variable to nil

    [frontView release], frontView = nil;

falconcreek
What do you mean by Candidate properties: ?
Code
Properties that I am guessing are retained.
falconcreek