5
\$\begingroup\$

I am writing a method to synchronize the local Core Data entities with a remote web service (in this case, Parse.com).

To update changed or created objects, I fetch all where the updatedAt date property is larger than the last local sync date. So far, so good.

But I am struggling with the deletion of local objects that have been removed on the server. My current approach is the following. This however seems quite expensive to me (\$O(r*l)\$ in the worst case, if I recall correctly). Is there any way to reduce the two loops?

PFQuery *idQuery = [PFQuery queryWithClassName:@"Product"];
[idQuery selectKeys:@[]];
NSArray *allRemote = [idQuery findObjects];
NSArray *allRemoteIds = [allRemote valueForKey:@"objectId"];

NSArray *allLocal = [Product all];
for(Product *p in allLocal) {
    BOOL shouldDelete = YES;
    for(NSString *remoteId in allRemoteIds) {
        if([remoteId isEqualToString:p.productId]) {
            shouldDelete = NO;
            break;
        }
    }

    if(shouldDelete) {
        [p delete];
    }
}
\$\endgroup\$
  • \$\begingroup\$ For what it's worth, two nested for-loops are not the end of the world, are they? \$\endgroup\$ – Sebastian Wramba Jan 27 '14 at 8:43
1
\$\begingroup\$

You can use NSPredicate:

NSArray* productsToDelete = [allLocal filteredArrayUsingPredicate:[NSPredicate predicateWithFormat:@"NOT (productId IN %@)", allRemoteIds]];
for(Product* product in productsToDelete) {
    [product delete];
}

This will for sure improve readability of your code. I'm not sure about performance, but I would assume that Apple's implementation of filteredArrayUsingPredicate should be fast enough. If you really want to know it, do some profiling.

You can also read great article about performance of collections in Objective-C:

http://www.objc.io/issue-7/collections.html

\$\endgroup\$
  • \$\begingroup\$ As a note of interest, from a performance stand point, forin loops do tend to be faster than filteredArrayUsingPredicate:. \$\endgroup\$ – nhgrif Jun 17 '14 at 0:28
0
\$\begingroup\$

That inner for-loop is basically equal to

shouldDelete = !([allRemoteIds containsObject:p.productId]);

i.e. if the product id exists in allRemoteIds then it should be deleted. The implementation of containsObject may have to go through the array of ids but it should in most cases quickly discard an object based on its hash which should make it faster. I also find it more readable than an explicit loop.

I'm not sure what the delete method does in your case but it's not safe to add or remove objects from an array while enumerating it. If that isn't the case for you then your code could can be simplified like this:

for (Product *product in allLocal) {
    BOOL shouldDelete = !([allRemoteIds containsObject:p.productId]);
    if(shouldDelete) {
        [p delete];
    }
}

Don't worry about the temporary variable. The compiler is going to remove that and it helps with readability.

If your delete operation is safe to run concurrently on many objects then it may be slightly faster to run it like this:

[allLocal enumerateObjectsWithOptions:NSEnumerationConcurrent
                           usingBlock:^(id obj, NSUInteger idx, BOOL *stop) {
    Product *p = obj;
    BOOL shouldDelete = !([allRemoteIds containsObject:p.productId]);
    if(shouldDelete) {
        [p delete];
    }
}];

You should measure and see if that makes a difference for you.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.