kastiglione / Parse-RACExtensions

ReactiveCocoa for Parse
MIT License
61 stars 9 forks source link

Ideal API for #7 #14

Open mhupman opened 10 years ago

mhupman commented 10 years ago

Have you thought about what kind of interface would be appropriate for #7? Methods like getDataStreamInBackgroundWithBlock:progressBlock: seem to complicate things in that they have multiple types of values that could be sent across the signal - (int) percentDone and (NSInputStream *) result in this case.

Here are some potential implementations I can see:

1) Send single values, require client to inspect object class - :-1:

[[myPFFile rac_getDataStreamInBackgroundWithProgress] subscribeNext:^(NSObject *obj) {
    // Progress
    if ([obj isKindOfClass:[NSNumber class]]){}
    // Result
    else if ([obj isKindOfClass:[NSInputStream class]]){}
}];

2) Send tuple values for progress-enabled signals, single values for data-only ones.

// With Progress
[[myPFFile rac_getDataStreamInBackgroundWithProgress] subscribeNext:^(RACTuple *tuple) {

    RACTupleUnpack(NSInputStream *stream, NSNumber *percentDone) = tuple;

    if (percentDone.intValue == 100) {}

    if (stream) {}
}];

// No Progress
[[myPFFile rac_getDataStreamInBackground] subscribeNext:^(NSInputStream *stream) {
}];

3) Support an optional out parameter for the progress signal. Bizarre since only one of the signals (probably the returned one) would kick off the work when subscribed to. :-1:

RACSignal *progressSignal;
[[myPFFile rac_getDataStreamInBackgroundWithProgress:&progressSignal] subscribeNext:^(NSInputStream *stream) {

}];

RAC(self, percentDone) = progressSignal;

2) seems to be the best of these choices IMO. I took a look at how this was implemented before 78edef702d1a52879c5e19bb97f741d8c605e8dc, but I don't think that interface differed much from option 1) as far as consumers are concerned.

Thoughts or other suggested implementations?

kastiglione commented 10 years ago

The previous implementation in 78edef7 returned a signal of signals, which consisted of exactly two signals, and sent in a static order. One of the signals would be a progress update signal, and the other would be a signal that sent just the NSData/NSStream.

To get the behavior of 1) from this compound signal, you could -flatten it.

To get the behavior of 2) you could use a custom -combineLatest operator like I've done in LabRAC.

To get the behavior of 3) you could maybe use -first, assuming the progress signal was sent first.

For the caller to get just one of the two signals, it's a matter of calling either take:1 or takeLast:1 and then calling either -flatten or -concat.

What do you think of this signal of signals approach?

mhupman commented 10 years ago

My only real concern about the signal of the signals approach is the lack of clarity about the values that will be sent along the signal. IMO, RAC's clarity suffers due to obj-c's lack of generics, and I wonder if the signal of signals approach adds more cognitive complexity to that (Now that I think about it, my 2nd suggestion is probably just as bad in this respect). On the other hand, seasoned RAC users are probably used to dealing with signals of signals anyway.

A potential compromise could be to split the category into two types - "simple" and "progress" variants. Simple variants would act just like the rest of the Parse-RACExtensions methods, e.g.

[[myFile rac_getData] subscribeNext:^(NSData *data){...}]

Progress variants would be suitable for advanced usage, and correspondingly, have the complex interface:

[[[myFile rac_getDataWithProgress] take:1] subscribeNext:^(NSNumber *progress){...}] [[[myFile rac_getDataWithProgress] takeLast:1] subscribeNext:^(NSData *data){...}]

The "progress" usage would basically be exactly the same as your original implementation. The "simple" implementations could be based off of the corresponding "progress" variants, so the total implementation complexity shouldn't be much worse than 78edef7.

kastiglione commented 10 years ago

:+1: Seems like the best of all worlds solution. Sprinkle in documentation with examples, and cough tests, and it should be quite approachable. Do you want to take a swing at it?

kastiglione commented 10 years ago

Actually, there's another approach for consideration: Make a synthetic Download class to encapsulate the fetched data and its progress. In this scenario -rac_getData* will send an instance of Download. This class could have properties for progress and data/stream, which can then be RACObserve'd as desired by the caller.

For example:

[[file rac_getData] flattenMap:^(Download *download) {
    return RACObserve(download, progress);
}];

I haven't thought much about this approach, particularly its pros and cons, so just putting it out there.