The right place to do things in Objective-C

I started toying around with the ObjectiveFlickr framework with the goal of creating a relatively simple iPhone map application showing geotagged flickr content within the current MKMapView region. I ran into threading related trouble before and now I have the feeling I am getting something fundamentally wrong in my architecture. Basically what I have is:

1. A MainViewController that creates and handles the MKMapView object and a button
2. Tapping the button calls a method that calls the Flickr API for geotagged photos within the current map extent.
3. The callback method for this API call iterates through the results and puts them in an NSMutableArray of FlickrImage objects. FlickrImage is a simple data class holding the flickr image location as a CLLocation, a NSURL pointing to the thumbnail, and a title NSString.

Code snippet for step 2:

-(void)actionSearchForTripodPhotos {
    if(currentBoundingBox == nil) {
        // TODO add a messagebox saying we're waiting for location info - or just lock the app until we're sure.
        return; 
    }
    NSString *dateTakenMinimumUNIXTimeStampString = [NSString stringWithFormat:@"%f",[[NSDate dateWithTimeIntervalSinceNow:-100000] timeIntervalSince1970]];
    OFFlick开发者_StackOverflow中文版rAPIRequest *flickrAPIRequest = [[OFFlickrAPIRequest alloc] initWithAPIContext:[CloudMadeMap101AppDelegate sharedDelegate].flickrAPIContext];
    [flickrAPIRequest setDelegate:self];
    NSString *flickrAPIMethodToCall = @"flickr.photos.search";
    NSString *bboxString = [NSString stringWithFormat:@"%f,%f,%f,%f",currentBoundingBox.bottomLeftLat ,currentBoundingBox.bottomLeftLon ,currentBoundingBox.topRightLat ,currentBoundingBox.topRightLon];
    NSLog(@"bounding box to be sent to flickr: %@",bboxString);
    NSDictionary *requestArguments = [[NSDictionary alloc] initWithObjectsAndKeys:FLICKR_API_KEY,@"api_key",[NSString stringWithFormat:@"%f",currentLocation.coordinate.latitude],@"lat",[NSString stringWithFormat:@"%f",currentLocation.coordinate.longitude],@"lon",dateTakenMinimumUNIXTimeStampString,@"min_upload_date",nil];
    [flickrAPIRequest callAPIMethodWithGET:flickrAPIMethodToCall arguments:requestArguments];
}

Code snippet for step 3:

- (void)flickrAPIRequest:(OFFlickrAPIRequest *)inRequest didCompleteWithResponse:(NSDictionary *)inResponseDictionary {
NSDictionary *photosDictionary = [inResponseDictionary valueForKeyPath:@"photos.photo"];
NSDictionary *photoDictionary;
FlickrImage *flickrImage;
for (photoDictionary in photosDictionary) {
  NSLog(@"photodictionary is %@",[photoDictionary description]);
  flickrImage = [[FlickrImage alloc] init];
  flickrImage.thumbnailURL = [[appDelegate sharedDelegate].flickrAPIContext photoSourceURLFromDictionary:photoDictionary size:OFFlickrThumbnailSize];
  flickrImage.hasLocation = TRUE; // TODO this is actually to be determined...
  flickrImage.ID = [NSString stringWithFormat:@"%@",[photoDictionary valueForKeyPath:@"id"]];
  flickrImage.owner = [photoDictionary valueForKeyPath:@"owner"];
  flickrImage.title = [photoDictionary valueForKeyPath:@"title"];
  [flickrImages addObject:flickrImage];
  [photoDictionary release];        
}
}

This all goes well. The API annoyingly does not return geolocation for each individual photo, so this requires another API call. I thought I might do this from within the FlickrImage class, but here it gets ugly:

• The MainViewController creates an instance of FlickrImage each iteration and stores it in the NSMutableArray.
• The FlickrImage instance calls the geolocation Flickr API asunchronously and should store the coordinate returned in the appropriate member variable.

I am pretty sure that this is not happening because I am getting

malloc: *** error for object 0x451bc04: incorrect checksum for freed object - object was probably modified after being freed.

sprinkled around my debugging output, and almost always a EXC_BAD_ACCESS but not consistently at the same point.

I am clearly doing something fundamentally wrong here, but what?

The error means exactly what it says: you have probably modified memory after freeing it, and the EXC_BAD_ACCESS indicates you are trying to access an array element that doesn't exist. I would check that your FlickrImage objects aren't being dealloc'ed by the time your are trying to call the geolocation method.

There is nothing in your design that stands out as being fundamentally flawed

When you iterate over dictionary there is no need to call [photoDictionary release]:

NSDictionary *photosDictionary =
      [inResponseDictionary valueForKeyPath:@"photos.photo"];
NSDictionary *photoDictionary;
FlickrImage *flickrImage;
for (photoDictionary in photosDictionary) {
  ... 
  [photoDictionary release];            

I think this is where your problem is.

When calling release and the object reaches ref count 0 it gets deallocated.

Because you were not supposed to do that, later on when the dictionary is released it sends release to each of its elements but you have possibly already deallocated them.

This is basic memory management in objective-c. Have a look at memory management and retain/release/autorelease stuff for more explanation.

@techzen - this is where I lack Xcode / gdb skills. How do I log those? That would provide useful insight indeed.

In the case of classes that inherent from NSObject you can simply have NSLog print the object directly.

NSLog(@"myObject=%@", myObjectInstance);

NSLog will call the instances debugDescription method which will usually print out something like:

<MyObjectClass 0x451bc04>

Some classes produce a lot more information but sometimes they don't provide the instances address so you have to print it directly:

NSLog(@"myObject's Address=%p",myObjectInstance);

The "%p" format specifier is the trick. You probably want to flesh it out a little like:

NSLog(@"<%@ %p>", [myObjectInstance class], myObjectInstance);
// prints <MyObjectClass 0x451bc04>

Put these statements in places where you create objects and when the error occurs you can see what objects failed by looking in the debugger console.

I think the primary problem is that your fast enumeration loop is not properly configured for a dictionary. Unlike arrays, fast enumeration on a dictionary returns only keys, not values. e.g.

NSDictionary *a=[NSDictionary dictionaryWithObjectsAndKeys:@"bob",@"bobKey",@"steve",@"steveKey",nil];
NSDictionary *b=[NSDictionary dictionaryWithObjectsAndKeys:@"bob1",@"bobKey",@"steve1",@"steveKey",nil];
NSDictionary *c=[NSDictionary dictionaryWithObjectsAndKeys:a,@"a",b,@"b",nil];
NSDictionary *d;
for (d in c) {
    NSLog(@"[d class]=%@,[d description]=%@",[d class],d);
    NSLog(@"[c valueForKey:d]=%@",[c valueForKey:d]);
}

prints:

[d class]=NSCFString,[d description]=a
[c valueForKey:d]={
    bobKey = bob;
    steveKey = steve;
}

[d class]=NSCFString,[d description]=b
 [c valueForKey:d]={
    bobKey = bob1;
    steveKey = steve1;
}

Note that even though d is defined as a NSDictionary it is still assigned as the string value of the keys.

You need to change:

NSDictionary *photoDictionary;
FlickrImage *flickrImage;
for (photoDictionary in photosDictionary) {

to something like:

NSString *dictKey;
NSDictionary *photoDictionary;
for (dictKey in photosDictionary) {
    photoDictionary=[photosDictionary valueForKey:dictKey];
    ...

and you don't need to release photoDictionary because you don't create a new object you just obtain a reference to it.