Refactoring into ReactiveCocoa

Question

So I have just recently started with ReactiveCocoa, and I figured the best way to learn would be just to jump right in and start refactoring some existing code that I have. I was wanting to get some critique and make sure I am heading in the right direction.

So in the app I am refactoring, I have a ton of code that goes like this:

[self.ff getArrayFromUri:@"/States?sort=name asc" onComplete:^(NSError *theErr, id theObj, NSHTTPURLResponse *theResponse) {
    if(!theErr) {
       //do something with theObj
    }
    else {
       //handle the error
    }
}];

I currently have this refactored in ReactiveCocoa like so:

-(void)viewDidLoad {
 //ReactiveCocoa
RACCommand *command = [RACCommand command];
RACSubject *subject = [RACSubject subject];
[[[command addSignalBlock:^RACSignal *(id value) {
    NSError *err;
    NSArray *array = [self.ff getArrayFromUri:@"/States" error:&err];
    err ? [subject sendError:err] : [subject sendNext:array];
    return [RACSignal empty];
}]switchToLatest]deliverOn:[RACScheduler mainThreadScheduler]];

[subject subscribeNext:^(NSArray *x) {
    [self performSegueWithIdentifier:kSomeSegue sender:x];
} error:^(NSError *error) {
    NSLog(@"the error = %@", error.localizedDescription);
}];

self.doNotLocation = [UIButton buttonWithType:UIButtonTypeCustom];
[self.doNotLocation setBackgroundImage:[UIImage imageNamed:@"BlackButton_small.png"] forState:UIControlStateNormal];
[[self.doNotLocation rac_signalForControlEvents:UIControlEventTouchUpInside] executeCommand:command];
RAC(self.doNotLocation.enabled) = RACAbleWithStart(command, canExecute);
RAC([UIApplication sharedApplication],networkActivityIndicatorVisible) = RACAbleWithStart(command, executing);    }

Is this about how I should be going about it, using the RACSubject, or is there a better way? This whole concept is new to me, as my only programming languages thus far have been Java and Objective-C, so this functional reactive way of thinking is throwing me off a bit.

Answer 1

Unfortunately, there are a couple problems with the code sample you presented:

1. The block passed to -addSignalBlock: is returning an empty signal. That should be a warning flag, since there are almost never junk return values. In this case, it means that the block performs its work synchronously. To avoid blocking the main thread, you should create a signal which does that work asynchronously, and return it.
2. The -switchToLatest and -deliverOn: are not doing anything. Most signal operators only do work when the resulting signal is subscribed to. In this case, it just disappears into the aether.

We can solve both of these problems at once. -addSignalBlock: returns a signal of the signals returned in the block. If we return something meaningful, it can be handled outside of that method.

First of all, this needs to be added to the top:

@weakify(self);

When @strongify(self) is used below, it will prevent a retain cycle. This is necessary because the RACCommand lives as long as self.

Now, the creation of the inner signals:

RACSignal *requestSignals = [command addSignalBlock:^(id value) {
    return [RACSignal start:^(BOOL *success, NSError **err) {
        @strongify(self);

        NSArray *array = [self.ff getArrayFromUri:@"/States" error:err];
        *success = (array != nil);

        return array;
    }];
}];

Within the block, this simply creates a signal which will invoke -getArrayFromUri:error: and pass back its results or an error, if one occurred. +start: will ensure that the work happens in the background.

Out of all of this, we get requestSignals, which is a signal of those created signals. This can completely replace the RACSubject used originally:

RACSignal *arrays = [[[requestSignals
    map:^(RACSignal *request) {
        return [request catch:^(NSError *error) {
            NSLog(@"the error = %@", error);
            return [RACSignal empty];
        }];
    }]
    flatten]
    deliverOn:RACScheduler.mainThreadScheduler];

First, we transform each inner signal to log, then ignore, errors. (It's a little complicated, but a RAC operator might be added to do it in the future.)

Then we flatten the signal of signals. The result, arrays, is a signal that passes through the values of all the inner signals. This is why we had to ignore errors – if any of them reached this point, we would stop getting all values from the inner signals, forever.

Finally, we "lift" the selector to invoke:

[self rac_liftSelector:@selector(performSegueWithIdentifier:sender:) withObjects:kSomeSegue, arrays];

This will resend -performSegueWithIdentifier:sender: whenever arrays sends a new value (which will be an NSArray returned from the network). You can think of it like invoking a method over time. This is better than a subscription because it simplifies side effects and memory management.

Answer 2

In my experience with the framework, I've found that there's very little reason to use RACSubject directly, especially for one-off signals like this. RACSubjects represent mutable signals, which you do not need in this case, and can actually increase the complexity of your code. It would be far better for you to return a vanilla signal (through +[RACSignal createSignal:]) inside that command block, then have the resulting request code make up the body of the signal:

[[[command addSignalBlock:^RACSignal *(id value) {
    //
    return [RACSignal createSignal:^RACDisposable *(id<RACSubscriber> subscriber) {
        //Request code here
            return nil;
    }];
}]switchToLatest]deliverOn:[RACScheduler mainThreadScheduler]]; 

Or, better still, you can refactor getArrayFromUri:error: to return a signal and get rid of that ternary statement:

 [[[command addSignalBlock:^RACSignal *(id value) {
     return [RACSignal createSignal:^RACDisposable *(id<RACSubscriber> subscriber) {
        //...
        [[self getArrayFromUri:@"/States"]subscribeError:^(NSError *error) {
            [subscriber sendError:error];
        } completed:^{
            [subscriber sendNext:array];
        }];
            return nil;
        }];
  }]switchToLatest]deliverOn:RACScheduler.mainThreadScheduler];

As for the problem of the subscription on the next line, those can be considered side effects of the signal, so we can make them explicitly so with the corresponding variants of do: applied to the signal for the command:

    [[[command addSignalBlock:^RACSignal *(id value) {
        return [RACSignal createSignal:^RACDisposable *(id<RACSubscriber> subscriber) {
            [[[[self getArrayFromUri:@"/States"]doError:^(NSError *error) {
                NSLog(@"the error = %@", error.localizedDescription);
                [subscriber sendError:err];
            }] doNext:^(NSArray *array) {
                [subscriber sendNext:array];
                [self performSegueWithIdentifier:kSomeSegue sender:array];
            }] subscribeCompleted:^{
                [subscriber sendCompleted];
            }];
            return [RACDisposable disposableWithBlock:^{
                 // Cleanup
            }];
        }];
    }]switchToLatest]deliverOn:[RACScheduler mainThreadScheduler]]; 

Finally, because commands work a differently from signals, the outermost operators will not be evaluated (thanks, @jspahrsummers), so you can remove them.

[command addSignalBlock:^RACSignal *(id value) {
            return [RACSignal createSignal:^RACDisposable *(id<RACSubscriber> subscriber) {
                [[[[self getArrayFromUri:@"/States"]doError:^(NSError *error) {
                    NSLog(@"the error = %@", error.localizedDescription);
                    [subscriber sendError:err];
                }] doNext:^(NSArray *array) {
                    [subscriber sendNext:array];
                    [self performSegueWithIdentifier:kSomeSegue sender:array];
                }] subscribeCompleted:^{
                    [subscriber sendCompleted];
                }];
                return [RACDisposable disposableWithBlock:^{
                     // Cleanup
                }];
            }];
        }];