1

So here is the background code needed to understand my problem. In my view controller class, I have a method which creates an array called levelStart, that array is then sent to my game class which intialized itself with that array, then I have a set of functions that use this array to "play the game". Here is the view controller code:

    NSMutableArray* levelStart = [[NSMutableArray alloc] init];
    NSMutableArray* levelFinish = [[NSMutableArray alloc] init];
    NSString* startString = [[NSString alloc] initWithString:@"9999999999988888888998888888899888888889988888888998888888899888888889911888888991188888899999999999"];
    NSString* finishString = [[NSString alloc] initWithString:@"9999999999988888811998888881199888888889988888888998888888899888888889988888888998888888899999999999"];
    int currentsquare;
    for (int i = 0; i < 100; i++) {
        currentsquare = [startString characterAtIndex:i] - '0';
        if (currentsquare == 1) {
            NSNumber* numb1 = [[NSNumber alloc] initWithInt:1];
            [levelStart addObject:numb1];
        }
        if (currentsquare == 2) {
            NSNumber* numb2 = [[NSNumber alloc] initWithInt:2];
            [levelStart addObject:numb2];
        }
        if (currentsquare == 3) {
            NSNumber* numb3 = [[NSNumber alloc] initWithInt:3];
            [levelStart addObject:numb3];
        }
        if (currentsquare == 4) {
            NSNumber* numb4 = [[NSNumber alloc] initWithInt:4];
            [levelStart addObject:numb4];
        }
        if (currentsquare == 8) {
            NSNumber* numb8 = [[NSNumber alloc] initWithInt:8];
            [levelStart addObject:numb8];
        }
        if (currentsquare == 9) {
            NSNumber* numb9 = [[NSNumber alloc] initWithInt:9];
            [levelStart addObject:numb9];
        }
    }
    for (int i = 0; i < 100; i++) {
        currentsquare = [finishString characterAtIndex:i] - '0';
        if (currentsquare == 1) {
            NSNumber* fnumb1 = [[NSNumber alloc] initWithInt:1];
            [levelFinish addObject:fnumb1];
        }
        if (currentsquare == 2) {
            NSNumber* fnumb2 = [[NSNumber alloc] initWithInt:2];
            [levelFinish addObject:fnumb2];
        }
        if (currentsquare == 3) {
            NSNumber* fnumb3 = [[NSNumber alloc] initWithInt:3];
            [levelFinish addObject:fnumb3];
        }
        if (currentsquare == 4) {
            NSNumber* fnumb4 = [[NSNumber alloc] initWithInt:4];
            [levelFinish addObject:fnumb4];
        }
        if (currentsquare == 8) {
            NSNumber* fnumb8 = [[NSNumber alloc] initWithInt:8];
            [levelFinish addObject:fnumb8];
        }
        if (currentsquare == 9) {
            NSNumber* fnumb9 = [[NSNumber alloc] initWithInt:9];
            [levelFinish addObject:fnumb9];
        }
    }
    [startString release];
    [finishString release];
 //NOTE: I took out the intialization of control and level, both which work fine.
    myGame = [[Game alloc] initLevel:level With:levelStart AndFinish:levelFinish WithRows:10 WithColumns:10 WithControl:control];

In my Game class this is the initalization method:

-(id)initLevel:(int)aLevel With:(NSMutableArray*)starter AndFinish:(NSMutableArray*)finisher WithRows:(int)rows WithColumns:(int)cols WithControl:(Coord)aControlSquare{
self = [super init];
if (self != nil){
    level = aLevel;
    squares = [[NSMutableArray alloc] init];
    squares = starter;
    finish = [[NSMutableArray alloc] init];
    finish = finisher;
}
return self;
}

Now the method where it keeps freezing on is in Game, which is triggered by UIgesture recognizer, here is the line it freezes on:

 if ([[self squareForCoord:test] intValue] == 8) {
 ...
 }

where squareForCord takes the test coord and returns the NSNumber at that coord. So I have made sure all my tests are inside bounds, and the point it freezes on is in bounds of my array. I believe I am initializing everything correctly but obviously not, this is definately not a problem with releasing something released so I figure it must be an initalization issue, please help.

Lou Franco
  • 87,846
  • 14
  • 132
  • 192
Peter P
  • 289
  • 1
  • 5
  • 17
  • 3
    Can you add a little more context around the like that crashes? Showing where test comes from and maybe the content of squareForCoord? You could also split this in 2 calls to see if it is [self squareForCoord:test] that crashes or [XXXX intValue]. Finally, this part seems odd: squares = [[NSMutableArray alloc] init]; squares = starter;. You allocate an array the replace it by another array. I think that your intention was to copy the array but your code is not doing that. – J_D Apr 12 '12 at 17:20
  • What is the full error message? – zaph Apr 12 '12 at 17:27
  • You should be able to use `NSZombie` to find out why: http://stackoverflow.com/questions/5386160/how-to-enable-nszombie-in-xcode – keegan3d Apr 12 '12 at 17:50
  • Okay so, first off squareForCoord is called on every part of the array because the info from every square must be displayed prior, and intValue is not the problem because an NSNumber should not have a problem with intValue. Now the array is set to squares in the initalization, because it must be displayed after it is initated. There is no error as in a crash but the game "pauses" and it says llbd in the debugger window and a green line comes up with EXC_BAD_ACCESS – Peter P Apr 12 '12 at 17:55

2 Answers2

2

There are some problems in your code, but I don't see an EXC_BAD_ACCESS source, but I have some advice on finding it.

Before that -- here are some other things you might want to fix

  1. If you initialize local string like this

    NSString* startString = @"9999999999988888888998888888899888888889988888888998888888899888888889911888888991188888899999999999";
    

    There is no need to release them. This makes it more likely that you will do it correctly.

  2. When iterating over the characters, don't hardcode a length. Use the string's length

    for (int i = 0; i < [startString length]; i++) {
    
  3. All of your if blocks like this

    if (currentsquare == 1) {
        NSNumber* numb1 = [[NSNumber alloc] initWithInt:1];
        [levelStart addObject:numb1];
    }
    

    Can be replaced by just doing this (but, read #4 first)

    NSNumber* numb1 = [[NSNumber alloc] initWithInt:currentsquare];
    [levelStart addObject:numb1];
    

    No need to check for each possible number

  4. That code above leaks, so really it should be this

    NSNumber* numb1 = [NSNumber numberWithInt:currentsquare];
    [levelStart addObject:numb1];
    

    Whenever you alloc, you probably need to release unless you are passing that responsibility to someone else. If you read the addObject docs, you will see that it calls retain, so it did not accept responsibility for your release. In this case numberWithInt returns an object that is autoreleased.

  5. This code

    squares = [[NSMutableArray alloc] init];
    squares = starter;
    

    immediately leaks the first array. You set squares to one array object and then a totally different one. The reference to the first one is lost. The first line should be deleted (and similarly with finish).

If this is a new project, then I would recommend you immediately convert it to ARC. This will make dealing with memory much easier.

However, if you can't for some reason, please read this blog I wrote that explains EXC_BAD_ACCESS:

http://loufranco.com/blog/files/Understanding-EXC_BAD_ACCESS.html

Briefly (after reading it) here is what I recommend you do

  1. Run an Analyze and fix every single error it finds. It uses the same algorithms that ARC uses, and it's very likely that every error it points out is a real problem (it will point out problem #4 and #5 above)

  2. Turn on zombies. I know you think you aren't sending a message to a released object, and there aren't any instances in your code above, but based on what I see, I think it's very likely that there are. A zombie is an object with a retain count of zero. When zombies are turned on, these objects are no longer deallocated and instead they just complain when you send a message to them.

If you can't find the problem after doing that, then check out the other suggestions in the blog.

Lou Franco
  • 87,846
  • 14
  • 132
  • 192
  • Thank you I am going to go through my code fixing the leaks, I will analyze right after and hopefully figure out the problem – Peter P Apr 12 '12 at 18:41
  • So I went through analyze, had a few errors fixed all but one, which I don't know, so in my initalize method there is another array isMovable, and i set it like so: isMovable = [self createIsMovable]; and createIsMovable I alloc an array, the one i want to return, should i autorelease it? – Peter P Apr 12 '12 at 20:02
  • Analyze doesn't know that the return is meant to be released by the caller. It gets that information from a naming convention -- call the message `newIsMovable` and it will understand. The caller is responsible for releasing it at some point. Or, leave it as is and autorelease the return. In that case, the caller must immediately retain. – Lou Franco Apr 12 '12 at 20:33
  • Points 3&4: Instead of `NSNumber* numb1 = [[[NSNumber alloc] initWithInt:currentsquare] autorelease]; [levelStart addObject:numb1];` just `[levelStart addObject:[NSNumber numberWithInt:currentsquare]]; Less code is more :-) – zaph Apr 12 '12 at 21:11
  • So in my initalization of the game I do something like isMovable = [[self createIsMovable] retain]; or isMovable = [self createIsMovable];[isMovable retain]; But also add the auto release in the return. – Peter P Apr 12 '12 at 21:32
  • Zaph that does exactly the same thing, does it not? – Peter P Apr 12 '12 at 21:41
  • The most common thing is to use properties to manage this. If you declare isMovable `@property (nonatomic, retain) isMovable` then `self.isMovable = [self createIsMovable];` will call retain and `createIsMovable` must autorelease. In your `dealloc` you call `self.isMovable = nil;` and that will release if it was set. NOTE: `self.isMovable = ...` is not the same as `isMovable = ...`, the first is a property (and calls retain as appropriate), the second is an instance variable and does nothing with regards to retain/release. – Lou Franco Apr 13 '12 at 12:05
0

First off, do you ever release levelStart and levelFinish arrays in your first class implementation? If so this is the clear issue as you are not retaining these objects after you instantiate your second class. My suggestion is to use properties to hold your arrays in the second class. Something like

 @property (nonatomic, retain)  NSMutableArray      *squares;
 @property (nonatomic, retain)  NSMutableArray      *finish;

In your declaration followed by:

@synthesize squares, finish;

In your .m file and then finally make the assignments like so in your initializer:

   self.squares = starter;
   self.finish = finisher;

Now you have a retained copy in the new class. Also I would consider using:

   myGame = [[Game alloc] initLevel:level With:[ levelStart autorelease ] AndFinish:[levelFinish autorelease] WithRows:10 WithColumns:10 WithControl:control];

for improved readability. Also, I would consider using ARC to avoid these problems.

Greg Price
  • 2,556
  • 1
  • 24
  • 33