5
votes

I have some code that downloads an image and assigns it within a block. The code currently works however I want to refactor it into a separate method, however after the refactoring I get a compilation error.

This is the original code which compiles and runs with the downloaded image being assigned successfully:

- (void) someMethod
{
   …
   MyObject* object = [[MyObject alloc] init];
   [self.objects addObject: object];

   NSString* graphRequest = [NSString stringWithFormat:@"%@%@%@",  @"https://graph.facebook.com/",
                              fbid,
                              @"/picture?type=square"];
    FBRequest *fbRequest = [FBRequest requestForGraphPath: graphRequest];
    [fbRequest startWithCompletionHandler:
     ^(FBRequestConnection *connection, id result, NSError *theError)
     {
         NSDictionary<FBGraphObject> *dict = (NSDictionary<FBGraphObject> *) result;
         if (dict)
         {
             NSString* urlAsString = [dict objectForKey:@"id"];
             if ([urlAsString length] > 0)
             {
                 NSURL *url = [NSURL URLWithString: urlAsString];
                 NSData *data = [NSData dataWithContentsOfURL:url];
                 object.image = [UIImage imageWithData:data];
             }
         }
     }];
}

If I refactor it to the following then I get a compilation error

- (void) someMethod
{
   …
   MyObject* object = [[MyObject alloc] init];
   [self.objects addObject: object];
   [self fetchFbImage: object.image forFbid:fbid];
}


- (void) fetchFbImage:(UIImage*) targetImage forFbid:(NSString*) fbid
{
    NSString* graphRequest = [NSString stringWithFormat:@"%@%@%@",  @"https://graph.facebook.com/",
                              fbid,
                              @"/picture?type=square"];
    FBRequest *fbRequest = [FBRequest requestForGraphPath: graphRequest];
    [fbRequest startWithCompletionHandler:
     ^(FBRequestConnection *connection, id result, NSError *theError)
     {
         NSDictionary<FBGraphObject> *dict = (NSDictionary<FBGraphObject> *) result;
         if (dict)
         {
             NSString* urlAsString = [dict objectForKey:@"id"];
             if ([urlAsString length] > 0)
             {
                 NSURL *url = [NSURL URLWithString: urlAsString];
                 NSData *data = [NSData dataWithContentsOfURL:url];
                 targetImage = [UIImage imageWithData:data];
             }
         }
     }];
}

The compilation error is the line assigning into targetImage, "Variable is not assignable (missing __block type specifier)".

Where should I add the __block type specifier? And why is there an issue after the refactoring but not before?

Thanks

2

2 Answers

4
votes

You seem to be confused over how parameters work in Objective-C.

In your original code you have:

MyObject* object = [[MyObject alloc] init];

which declares object as a local variable of the method. Then within the block you write:

object.image = [UIImage imageWithData:data];

So your block references the local variable object and (as you have no __block attribute on the declaration) the block gets a constant copy of the contents of variable. This is fine because you are not changing what is in object, which is a reference to your MyObject instance, but are calling a method on that instance which changes the internal state of that instance.[*]

Now let's look at your refactoring. You remove a chunk of your someMethod's code and place it in a new method fetchFbImage. In someMethod you call fetchFbImage:

[self fetchFbImage:object.image forFbid:fbid];

This passes the current value of object.image to the method it does not pass the property in such a way that it can be assigned to in fetchFbImage. The type of the argument targetImage is UIImage * - a reference to an UIImage - it is not "a property of type UIImage *" - you cannot have arguments of such a type, properties cannot be passed around only their values or reference to objects which have properties.

When a method is called each parameter is effectively a local variable which is initialised to the argument value passed, so the above method call effectively performs:

UIImage *targetImage = object.image;

where targetImage is a local variable in the scope of fetchFbImage and object is the local variable in the scope of someMethod.

Now within the block inside of fetchFbImage you write:

targetImage = [UIImage imageWithData:data];

This is wrong for two reasons:

  1. You cannot assign to targetImage within the block. This is a local variable belonging to fetchFbImage, that local variable is not attributed with __block, and so the block has a constant copy of it - and you cannot assign to constants. This is why the compiler issues the error message.
  2. However your issue is bigger than this, you are assuming that an assignment to fetchFbImage's local variable targetImage will some how invoke the property object.image - and there is no way it can do that. targetImage is just a local variable which was initialised with the value of object.image by the method call.

The only way to fix this is to pass fetchFbImage a reference to your MyObject instance and then within the block inside of fetchFbImage to assign to the image property of that object just as your pre-refectored code did.

So your code will look something like:

- (void) fetchFbImage:(MyObject *)targetObject forFbid:(NSString *)fbid
{
   ...
   targetObject.image = [UIImage imageWithData:data];
   ...
}

...

[self fetchFbImage:object forFbid:fbid];

HTH

Addendum

Seeing your comment on another answer it appears you would like fetchFbImage to have no knowledge of MyObject and be able to fetch images regardless of where they will be referenced from.

A simple way to do this is to follow the same design you have for FBRequest - use a completion handler. For convenience define a type for the completion block:

typedef void (^ImageConsumer)(NSImage *image);

Now define your method to take one of these:

- (void) fetchFbImageForFbid:(NSString *)fbid completionHandler:(ImageConsumer)handler

In your block within fetchFbImageForFbid pass the image to the handler:

handler([UIImage imageWithData:data]);

And in the call in someMethod pass a block which assigns the value to your property:

[self fetchFbImageForFbid:fbid
        completionHandler:^(NSImage *image) { object.image = image; }
];

[*] If this is confusing think of the reference as the address of a house. The address is constant, how many people are in the house is not. You can tell someone "go to this address, there is a great party on" - the contents ("state") of the house changes, its address does not.

1
votes

in your first method (with out refactoring), you can set the image to object becuase you have reference to the object, so you did

object.image = [UIImage imageWithData:data];

But after refactoring , you can't set image in the way you are doing,you should send the object also.

- (void) someMethod
{
   …
   MyObject* object = [[MyObject alloc] init];
   [self.objects addObject: object];
   [self fetchFbImageForObj:object forFbid:fbid];
}


- (void) fetchFbImageForObject:(MyObject*)object forFbid:(NSString*) fbid
{
    NSString* graphRequest = [NSString stringWithFormat:@"%@%@%@",  @"https://graph.facebook.com/",
                              fbid,
                              @"/picture?type=square"];
    FBRequest *fbRequest = [FBRequest requestForGraphPath: graphRequest];
    [fbRequest startWithCompletionHandler:
     ^(FBRequestConnection *connection, id result, NSError *theError)
     {
         NSDictionary<FBGraphObject> *dict = (NSDictionary<FBGraphObject> *) result;
         if (dict)
         {
             NSString* urlAsString = [dict objectForKey:@"id"];
             if ([urlAsString length] > 0)
             {
                 NSURL *url = [NSURL URLWithString: urlAsString];
                 NSData *data = [NSData dataWithContentsOfURL:url];
                 object.image = [UIImage imageWithData:data];
             }
         }
     }];
}

try it.If any error or something,comment below.