HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Comparing IDs in two arrays

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
idsarrayscomparingtwo

Problem

I have two arrays that I need to compare if their IDs are equal. This is how I am currently doing it:

PFQuery *earnedQuery = [PFQuery queryWithClassName:@"EarnedAchievement"];
[earnedQuery whereKey:@"user" equalTo:[PFUser currentUser]];
[earnedQuery findObjectsInBackgroundWithBlock:^(NSArray *objects, NSError *error) {

    for (Achievement *achievement in allAchievements) {

        for (PFObject *earnedAchievement in objects) {

            if ([achievement.id isEqualToString:earnedAchievement[@"achievmentId"]]) {

                //user has earned this achievement
                achievement.earned = [NSNumber numberWithBool:YES];
                achievement.earnedDate = earnedAchievement[@"earnedOn"];

            }

        }

    }

    [context save:&error];

}];


Is there a better or more efficient way to do this outside of the two for in loops?

Solution

The first thing we can do to improve the efficiency of this code is to remember to check for an error and make sure that the returned objects array returns anything before we do anything. Generally, code from Apple libraries would guarantee our objects array to be empty if there were any sort of error, but some libraries don't make that guarantee.

We can also store our allAchievements locally as an NSDictionary rather than an array. If allAchievements is an NSDictionary, our code looks something more like this:

[earnedQuery findObjectsInBackgroundWithBlock:^(NSArray *objects, NSError *error) {
    for (PFObject *earnedAchievement in objects) {
        Achievement *achievement = allAchievements[earnedAchievement[@"achievmentId"]];
        if (achievement) {
            achievement.earned = @YES;
            achievement.earnedDate = earnedAchievement[@"earnedOn"];
        }
    }
    [context save:&error];
}];


Now we eliminate the nested loop. And we only run through the outer loop once per earnedAchievement (which may be zero times).

Also, keep in mind that everything within the findObjectsInBackgroundWithBlock: is executing in the background.

Some other comments...

I would eliminate the use of magic strings. Define all of your keys as constants somewhere. This serves multiple purposes.

  • If we ever decide to actually change the key, it's changed everywhere all at once.



  • Typing magic strings is prone to typos, where as using a variable won't compile if we've made a typo (unless we typo into another variable name).



  • Typing magic strings won't auto-complete (which makes typos more likely, see point 2), but constant variables will.

Code Snippets

[earnedQuery findObjectsInBackgroundWithBlock:^(NSArray *objects, NSError *error) {
    for (PFObject *earnedAchievement in objects) {
        Achievement *achievement = allAchievements[earnedAchievement[@"achievmentId"]];
        if (achievement) {
            achievement.earned = @YES;
            achievement.earnedDate = earnedAchievement[@"earnedOn"];
        }
    }
    [context save:&error];
}];

Context

StackExchange Code Review Q#90857, answer score: 3

Revisions (0)

No revisions yet.