0

This is a pretty simple concept, but I'm not getting the results I'm wanting. I have an NSMutableArray that is populated with NSArrays, I want to loop through that NSMutableArray and remove certain NSArrays based on a key-value pair. My results have many of the NSArrays that I should be removing and I think it has something to do with the count of the NSMutableArray and the int I declare in the For Loop.

Here is my code: (restArray is the NSMutableArray)

for (int i=0; i<restArray.count; i++) {
    NSArray *array = restArray[i];
    if ([[array valueForKey:@"restaurant_status"] isEqualToString:@"0"]) {
        [restArray removeObjectAtIndex:i];
    }
}

Does someone know what I am doing wrong here?

Improve this question
4
  • removeObjectAtIndex at this context removes the NSArray at that index instead of removing the object in that NSArray . Pleae clarify what you need to remove. Commented Oct 19, 2015 at 15:43
  • I did need to remove the entire NSArray at that index Commented Oct 19, 2015 at 15:46
  • You use NSArray for key/value storage? Maybe NSDictionary is better? Commented Oct 19, 2015 at 15:50
  • 1
    Your code will work if you iterate in reverse. Commented Oct 19, 2015 at 16:23

3 Answers 3

Reset to default
4

It is not recommended to modify an array on what are you currently iterating.

Lets create a tmp array, and reverse your logic.

NSMutableArray * tmpArray = [NSMutableArray array];
for (int i=0; i<restArray.count; i++) {
    NSArray *array = restArray[i];
    if (![[array valueForKey:@"restaurant_status"] isEqualToString:@"0"] {
        [tmpArray addObject:array];
    }
}

So at the end of the iteration, you should end up with tmpArray having the arrays you needed.

Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

Thats solid logic, then once I have tmpArray I can replace restArray with tmpArray. Thanks!
FYI - don't use int, use NSUInteger for the loop.
@rmaddy really? I've always used int for loops, whats the difference there?
You always want to use the proper data type. Look at the data for NSArray count and objectAtIndex.
4

Use NSPredicate:

NSArray *testArray = @[@{@"restaurant_status" : @"1"}, @{@"restaurant_status" : @"0"}];
NSArray *result = [testArray filteredArrayUsingPredicate:[NSPredicate predicateWithFormat:@"(restaurant_status == %@)", @"1"]];
Improve this answer

Comments

3

When you remove an element all the elements past it shift down by one, e.g. If you remove the element at index 3 then the element previously at index 4 moves to index 3.

Every iteration you increase the index by one.

Combine the above two and you see that when you remove an element your code skips examining the following element.

The simple solution is to reverse the order of the iteration:

for (int i = restArray.count - 1; i >= 0; i--)

and then your algorithm will work.

Addendum

You can safely ignore this addendum if your arrays contain < 2^32 elements and you use Clang or GCC (and most other C compilers).

It has been raised in the comments that this answer has a problem if the array has 0 elements in it. Well yes & no...

First note that the code in the question is technically incorrect: count returns an NSUInteger which on a 64-bit machine is a 64-bit unsigned integer, the loop variable i is declared as an int which is 32-bit signed. If the array has more than 2^31-1 elements in it then the loop is incorrect.

Most people don't worry too much about this for some reason ;-) But let's fix it:

for (NSInteger i = restArray.count - 1; i >= 0; i--)

Back to the problem with an empty array: in this case count returns unsigned 0, C standard arithmetic conversions convert the literal 1 to unsigned, the subtraction is done using modular arithmetic, and the result is unsigned 2^64-1.

Now that unsigned value is stored into the signed i. In C converting from signed to unsigned of the same type is defined to be a simple bit-copy. However converting from unsigned to signed is only defined if the value is within range, and implementation defined otherwise.

Now 2^64-1 is greater than the maximum signed integer, 2^32-1, so the result is implementation defined. In practice most compilers, including Clang and GCC, choose to use bit-copy, and the result is signed -1. With this the above code works fine, both the NSInteger and the int (if you've less than 2^32-1 elements in your array) versions.

What the comments raise is how to avoid this implementation-defined behaviour. If this concerns you the following will handle the empty array case correctly with ease:

for (NSUInteger i = restArray.count; i > 0; )
{
   i--; // decrement the index
   // loop body as before
}

If the array is empty the loop test, i > 0, will fail immediately. If the array is non-empty i, being initialised to the count, will start as one greater than the maximum index and the decrement in the loop will adjust it - effectively in the loop test i contains the number of elements left to process and in the loop body after the decrement contains the index of the next element to process.

Isn't C fun (and mathematically incorrect by definition)!

Improve this answer

4 Comments

One danger here. If restArray.count is zero, you will loop over a very large range. When I do this I write the loop as for (NSInteger i = restArray.count; i > 0; i--). Then any index access inside the loop is done as restArray[i - 1].
@rmaddy - if count == 0 then i == -1 and i >= 0 fails before the first iteration. (Though we've got signed and unsigned types here that doesn't cause an issue.)
But count is NSUInteger so count - 1 gives a very large value. Then the large value is assigned to the int.
@rmaddy - I think you are both right in theory and wrong in practice, but maybe you disagree. Added an addendum to explain. Isn't C fun :-)

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.