CODEWITHSUNDEEP
objective-c
Gil
Gil

Reputation: 103

Program Crash - probably a memory retain misuse

I have an array of Objective-C objects that is to be sorted on a field of the object. I use a simple selection sort since the array is small. The sort works and the correct data is in the array, but after adding about 3 or 4 objects to the array and resorting each time it crashes. Any help would be appreciated. The error is EXC_BAD_ACCESS Thanks in advance. The code follows:

TopTenDataClass *temp1 = [[TopTenDataClass alloc] init];
TopTenDataClass *temp2 = [[TopTenDataClass alloc] init];
for (int i = 0; i < [topTenDataArray count]; i++)
{
    int minIndex = i;
    for (int j = i; j < [topTenDataArray count]; j++)
    {
        temp1 = [topTenDataArray objectAtIndex:minIndex];
        temp2 = [topTenDataArray objectAtIndex:j];
        if ( temp2.timeInSeconds < temp1.timeInSeconds)
            minIndex = j;
    }
    [topTenDataArray exchangeObjectAtIndex:minIndex withObjectAtIndex:i];
}
[temp2 release]; [temp1 release];

Upvotes: 0

Views: 107

Answers (1)

aroth
aroth

Reputation: 54806

The issue is that inside of your loop you are changing the values of temp1 and temp2. When you release them after the loop completes, you are not releasing the objects that you created before the loop started. Instead you are attempting to release some other object that you did not alloc/init (in this part of the code). Probably that is what is causing your crash.

I'd suggest trying something like:

TopTenDataClass *temp1 = nil;
TopTenDataClass *temp2 = nil;
for (int i = 0; i < [topTenDataArray count]; i++)
{
    int minIndex = i;
    for (int j = i; j < [topTenDataArray count]; j++)
    {
        temp1 = [topTenDataArray objectAtIndex:minIndex];
        temp2 = [topTenDataArray objectAtIndex:j];
        if ( temp2.timeInSeconds < temp1.timeInSeconds)
            minIndex = j;
    }
    [topTenDataArray exchangeObjectAtIndex:minIndex withObjectAtIndex:i];
}

You don't need to assign a new object instance to temp1 and temp2 before the loop, because you just overwrite their values inside your loop.

Do also note that if topTenDataArray has only a single element in it your code will call [topTenDataArray exchangeObjectAtIndex:0 withObjectAtIndex:0], which may also be a problem (the API docs don't say that you can't exchange an object with itself, but they don't say that you can, either).

Upvotes: 1

Related Questions