NSDate Comparison using for and if statements

Question

My app is attempting to determine if todays date is in between an object in an array's start date and end date, and would then remove that object from the array. The code successfully removes objects from the array, but only every other object (where all of them should be removed).

for (int i = 0; i < [AdDataArray count]; i++) {
    NSDate *dateStart = [self adStartDateAtIndex:i];
    NSDate *dateEnd = [self adEndDateAtIndex:i];
        if (([dateNow earlierDate:dateStart]) || ([dateNow laterDate:dateEnd]))
        {
        [AdDataArray removeObjectAtIndex:i];
        }
i++;
}

If a second set of eyes could go over my code and see what I am doing wrong, that would be greatly appreciated!

@JoshCaswell Not a dupe - the problem here is with the loop, not the date comparison. — rmaddy, Jul 19 '13 at 19:58
The comparison looks bad too... `[a earlierDate: b]` returns the earlier of `a` and `b` not a BOOL indicating that one is earlier than the other. (Likewise with `laterDate:` but in reverse.) Assuming `dateStart`, `dateEnd` and `dateNow` are all non-nil, all items will be removed, AFAICT. — ipmcc, Jul 19 '13 at 20:22
@ipmcc You are correct! I have just tried to add an object that should not be removed, was removed. I should instead try a comparison like, `if ((([dateNow compare:dateStart] =/= NSOrderedDescending) || ([dateNow compare:dateStart] =/= NSOrderedSame)) && (([dateNow compare:dateEnd] =/= NSOrderedAscending) || ([dateNow compare:dateEnd] =/= NSOrderedSame))) {`? — VonKoob, Jul 19 '13 at 20:25
This would also do it: `if (dateNow == [dateNow earlierDate:dateStart] || dateNow == [dateNow laterDate:dateEnd])` This is to say, "If now is the earlier of now and the start, or if now is the later of now and the end, remove it." — ipmcc, Jul 19 '13 at 20:54

rmaddy · Accepted Answer · 2013-07-19T20:21:57.587

2

The problem is with your loop counter. You delete an item and then do an extra increment of i.

When ever I need to do a similar type of loop I do it backwards:

for (NSUInteger i = AdDataArray.count; i > 0; i--) {
    NSDate *dateStart = [self adStartDateAtIndex:i - 1];
    NSDate *dateEnd = [self adEndDateAtIndex:i - 1];
    if (([dateNow earlierDate:dateStart]) || ([dateNow laterDate:dateEnd]))  {
        [AdDataArray removeObjectAtIndex:i - 1]; 
    }
}

The reason to go from AdDataArray.count to > 0 instead of AdDataArray.count - 1 to >= 0 is because if count is 0, using count - 1 causes a wrap since count is unsigned.

edited Jul 19 '13 at 20:21

answered Jul 19 '13 at 19:58

rmaddy

314,917
42
532
579

I never thought of working backwards as you have suggested, so I have gone ahead and changed my code accordingly. However, my app throws a SIGABRT when it attempts the line `[AdDataAray removeObjectAtIndex:i];` due to an `index 8 beyond bounds[0..7]`. Would I need to change it to `[AdDataAray removeObjectAtIndex:i-1];`? – VonKoob Jul 19 '13 at 20:11
Yes, sorry. Missed that. Except for the loop itself, all references to `i` should be `i - 1`. I'll update my answer. – rmaddy Jul 19 '13 at 20:21

score 0 · Answer 2 · answered Jul 19 '13 at 19:58

You have two i++'s. Remove the one at the end.

for (int i = 0; i < [AdDataArray count]; i++) {
    NSDate *dateStart = [self adStartDateAtIndex:i];
    NSDate *dateEnd = [self adEndDateAtIndex:i];
    if (([dateNow earlierDate:dateStart]) || ([dateNow laterDate:dateEnd]))
    {
        [AdDataArray removeObjectAtIndex:i];
    }
}

From developer.apple.com,

To fill the gap, all elements beyond index are moved by subtracting 1 from their index.

So actually, you might have to add i-- inside your if statement, since i will get auto incemented even if you remove an element.

NSDate Comparison using for and if statements

2 Answers2