2

I'm using Objective-C for this question but this is not really language specific.

I have following method in my User class,

+(BOOL)canPerform:(NSString *)string
    withCompletion:(void(^)(BOOL success,NSError *error))block;

In my ViewController

-(void)performTask{

        if([User canPerform:@"My String" withCompletion:^(BOOL success, NSError *error) {
            if (success) {
            NSLog(@"Task success!");

            }
            else{
            NSLog(@"Task Failed with error : %@,error.localizedDescription");
            }
        }])
        {
            NSLog(@"Can perform task");

        }
        else{
            NSLog(@"Can not perform task");

        }    
    }

It's not necessary to mention what task I'm performing. That's not really my question.

My questions are:

  1. Is this a good programming practice? Using a BOOL method that accepts a block, as the condition for an if statement ?

  2. This method [User canPerform] does two things. First thing is, it checks if it can perform this specific task and if it can, it does perform that task. But method name does not reflect this. Method name is canPeform. I could use ifCanThenPeformThisTask but that sounds strange and doesn't feel like a BOOL method. What is the best approach to name this method?

Rukshan
  • 7,902
  • 6
  • 43
  • 61

2 Answers2

4

It can be better. The name of the method is misleading and the code is hard to read because of the number of brackets and nesting everywhere. It would be better as something like:

+(BOOL)checkAndPerform:(NSString *)string
        withCompletion:(void(^)(BOOL success,NSError *error))block;

and used as:

-(void)performTask {
    BOOL available = [User checkAndPerform:@"My String" withCompletion:^(BOOL success, NSError *error) {
        if (success) {
            NSLog(@"Task success!");
        } else {
            NSLog(@"Task Failed with error : %@,error.localizedDescription");
        }
    }];

    if(available)
    {
        NSLog(@"Can perform task");
    } else {
        NSLog(@"Can not perform task");
    }    
}

this is now clear that we are both checking potential and executing if available, and it clearly separates that action from the resulting action based on the availability of execution.

Wain
  • 118,658
  • 15
  • 128
  • 151
3

You can find some Foundation methods, that do the similar job, bit with slightly different method name. To keep the method signature you currently have and to achieve better comprehending - rename your method to didPerform. So you method would look like this:

- (void)performTask {
    void(^completion)(BOOL, NSError *) = ^(BOOL success, NSError *error) {
        if (success) {
            // app logic
        } else {
            // app logic
        }
    };
    NSError *error = nil;

    if ([User didPerform:@"My String" error:&error withCompletion:completion(BOOL success, NSError *error)]) {
        NSLog(@"DID perform");
    } else {
        NSLog(@"some kind of error: %@", error.localizedDescription);

    }
}
Soberman
  • 2,556
  • 1
  • 19
  • 22