2
\$\begingroup\$

Does this looks ok to everyone?

NSFetchRequest *oldFetchRequest = [[NSFetchRequest alloc] init];
NSEntityDescription *oldEntryEntity = [NSEntityDescription entityForName:@"Entry"
                                                  inManagedObjectContext:oldContext];

[oldFetchRequest setEntity:oldEntryEntity];

int numberOfEntries = [oldContext countForFetchRequest:oldFetchRequest error:nil];

int batchSize = 10;
[oldFetchRequest setFetchBatchSize:10];
int offset = 0;

while (numberOfEntries - offset > 0) {

    [oldFetchRequest setFetchOffset:offset];
    NSError *error;
    NSArray *entries = [oldContext executeFetchRequest:oldFetchRequest error:&error];

    for (NSManagedObject *entry in entries) {
        Entry *newEntry = [NSEntityDescription insertNewObjectForEntityForName:@"Entry"
                                                        inManagedObjectContext:newContext];

        [newEntry setupCreationDate:[entry valueForKey:@"creationDate"] withSave:NO];
        newEntry.entryID = [entry valueForKey:@"entryID"];

        NSMutableOrderedSet *newMediaSet = [[NSMutableOrderedSet alloc] init];

        NSOrderedSet *mediaSet = [entry valueForKey:@"media"];

        int i = 0;

        for (NSManagedObject *media in mediaSet) {

            Media *newMedia = [NSEntityDescription insertNewObjectForEntityForName:@"Media"
                                                            inManagedObjectContext:newContext];

            newMedia.positionInEntry = [NSNumber numberWithDouble:i + 1]; //Potentially needs changing
            newMedia.mediaID = [Entry generateString];

            MediaImageData *imageData = [NSEntityDescription insertNewObjectForEntityForName:@"MediaImageData"
                                                                      inManagedObjectContext:newContext];

            if ([newMedia.type isEqualToString:@"Image"]) {
                imageData.data = [media valueForKey:@"originalImage"];;
            }
            else if ([newMedia.type isEqualToString:@"Movie"]) {
                NSURL *movieURL = [NSURL URLWithString:newMedia.movie];
                MPMoviePlayerController *moviePlayer = [[MPMoviePlayerController alloc] initWithContentURL:movieURL];
                [moviePlayer stop];
                UIImage *screenshot = [moviePlayer thumbnailImageAtTime:0.0 timeOption:MPMovieTimeOptionNearestKeyFrame];
                [moviePlayer stop];

                imageData.data = UIImageJPEGRepresentation(screenshot, 1.0);
            }

            newMedia.imageData = imageData;

            newMedia.entry = newEntry;
            [newMediaSet addObject:newMedia];

            i++;
        }

        newEntry.media = newMediaSet;


    }

    [newContext save:&error];

    offset = offset + batchSize;
}
\$\endgroup\$
2
\$\begingroup\$

The first big problem I see with this code is here:

int i = 0;

for (NSManagedObject *media in mediaSet) {
    // stuff
    newMedia.positionInEntry = [NSNumber numberWithDouble:i + 1];
    // more stuff
    i++;
}

First of all, if positionInEntry is a property intended to hold some sort of index or something for your Media class, then it should be of the same type that collections tend to use for their indexing: NSUInteger. Although... the object itself probably doesn't need to know what position it is in the array, and as this could change, it's probably best to just not have this property at all.

Next, i is defined as an int, and 1 is an int literal, so we're creating the NSNumber in the wrong way. We should be doing this (if we're going to continue to keep this positionInEntry property):

newMedia.positionInEntry = [NSNumber numberWithInt:i + 1];

Finally, we really shouldn't be doing this in this manner at all. If we want to know what index we're working with, we should use a traditional for loop.

for (int i = 0; i < [mediaSet count]; ++i) {
    NSManagedObject *media = mediaSet[i];
    // stuff
    newMedia.positionInEntry = @(i+1);
    // more stuff
}

However, there's a slightly better option.

It is good to use a forin loop where we can, because this actually is faster than a traditional for loop in Objective-C. We still don't want to rely on i however, for a couple of reasons, right now, I'll say, the most important of which is readability. But there is this option:

for (NSManagedObject *media in mediaSet) {
    // stuff

    NSUInteger index = [mediaSet indexOfObject:media];

    newMedia.positionInEntry = @(index + 1);

    // more stuff
}

We can call indexOfObject: on an NSArray and it will return the first index it finds that object at. As a note, this method returns NSNotFound if the object isn't found in the array, but in a forin loop, this should hopefully never be the case.

\$\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.