CODEWITHSUNDEEP
iphoneobjective-cnstimer
miniman
miniman

Reputation: 47

NSTimer invalidate not working

I am trying to create an explosion on the iphone screen which gets bigger fast, then goes away. Why is this timer not stopping?

NSTimer *explosion = [NSTimer scheduledTimerWithTimeInterval:0.1 target:self selector:@selector(explosion) userInfo:nil repeats:YES];



-(void)explosion {
    image.image = [UIImage imageNamed:@"explosion.png"];
    expsize = expsize + 2.5;
    image.frame = CGRectMake(image.frame.origin.x, image.frame.origin.y, expsize, expsize);
    if (expsize > 60) {
        NSLog(@"%f",expsize);
        [explosion invalidate];
        explosion = nil;
    }
}

Upvotes: 0

Views: 2376

Answers (4)

jscs
jscs

Reputation: 64002

It's hard to be certain, because it's not clear whether this is exactly your code, but you have two variables named explosion, and one of them has an NSTimer assigned to it; the other (which seems to be an ivar) is nil.

// Variable local to whatever method this is in
NSTimer *explosion = [NSTimer scheduledTimerWithTimeInterval:0.1...

if (expsize > 60) {
    NSLog(@"%f",expsize);
    // Other variable named "explosion" does not exist.
    // This is an ivar? Has not been set.
    [explosion invalidate];

Assuming you've got explosion declared as a property (and there is no reason not to), you should fix this by using the setter when you create the timer:

[self setExplosion:[NSTimer scheduledTimerWithTimeInterval:...]];

Now the ivar has the timer instance and you can use it to invalidate the timer.

Also, note that your timer's method is incorrect; it must take one parameter which is a pointer to the timer. You can also use this pointer to invalidate the timer when it fires.

- (void) fireExplosion: (NSTimer *)tim {

    //...
    if( expsize > 60 ){
        [tim invalidate];
        //...

    }
}

Finally, you have one last naming problem; if your property is called explosion, the convention in Cocoa is that the accessor should have the same name, but you have used explosion for the method that your timer calls. This could cause hard-to-track problems later. You should rename the timer method as I have here, using a verb indicating that something is happening.

Upvotes: 0

Hot Licks
Hot Licks

Reputation: 47699

I'd suggest that you use the form of selector that the NSTimer doc calls for: - (void)timerFireMethod:(NSTimer*)theTimer. The you can invalidate "theTimer" and be sure you're invalidating the right one.

Also, of course, if "explosion" is declared as a property, then there will be two methods in the class named "explosion", and no real clue as to which one is getting called.

Upvotes: 0

Joe
Joe

Reputation: 57169

If you are declaring explosion how you posted in your example then you are shadowing your instance variable explosion. As a word of advice you should use a naming convention for instance variables such as an underscore prefix. Now keeping track of the timer is not required if you only invalidate it after it fires. You could just take an extra parameter on the explosion method that would be the timer explosion:(id)timer. Otherwise you can do the following.

@interface X : NSObject
{
    NSTimer *_explosion;
}
@end

And when you go to declare it in your code do the following

...
[_explosion invalidate];
[_explosion release];
//There is a whole 'nother debate on whether or not to retain a scheduled timer
//but I am a stickler for ownership so remember to release this in dealloc
_explosion = [[NSTimer scheduledTimerWithTimeInterval:0.1 
                                               target:self 
                                             selector:@selector(explosion) 
                                             userInfo:nil 
                                              repeats:YES] retain];
...

-(void)explosion {
    image.image = [UIImage imageNamed:@"explosion.png"];
    expsize = expsize + 2.5;
    image.frame = CGRectMake(image.frame.origin.x, image.frame.origin.y, expsize, expsize);
    if (expsize > 60) {
        NSLog(@"%f",expsize);
        [_explosion invalidate];
        [_explosion release];
        _explosion = nil;
    }
}

Upvotes: -1

Felix
Felix

Reputation: 35384

You are most likely invalidating the wrong timer.

You create a local variable named explosion that has the same name as the instance variable. Avoid declaring instance variables and local variables with the same name!

Upvotes: 3

Related Questions