uber-archive / ohana-ios

Contacts simplified. This project is deprecated and not maintained.
MIT License
362 stars 40 forks source link

Proposed modification/fix of PostProcessorProtocol #32

Closed adam-zethraeus closed 8 years ago

adam-zethraeus commented 8 years ago

Current protocol:

@protocol OHContactsPostProcessorProtocol <NSObject>

/**
 *  Signal to be fired after the contacts post processor has finished its work (see signal definition)
 */
@property (nonatomic, readonly) OHContactsPostProcessorFinishedSignal *onContactsPostProcessorFinishedSignal;

/**
 *  The main method of any post processor, this method does the actual post processing on the contact data
 *
 *  @discussion The post processor may add, remove, or modify contacts passed into it.
 *
 *  @param preProcessedContacts The contacts to be run through the post processor
 *
 *  @return The contacts processed by the post processor
 */
- (NSOrderedSet<OHContact *> *_Nullable)processContacts:(NSOrderedSet<OHContact *> *)preProcessedContacts;

@end

The onContactsPostProcessorFinishedSignal here is never used in the Data Source. It also seems redundant to expect implementers to remember to fire this signal at the end of the synchronous call to processContacts (which already returns the same result).

maxwellE commented 8 years ago

Strange, I swore we used the post processor signal to note if post processing was complete

adam-zethraeus commented 8 years ago

Yeah, i'm pretty sure we did once upon a time.

side thoughts: There's a fair argument for using them to allow the consumer to control what thread to do processing on, but if we do that we can't have the PostProcessor returning values. And imho, that's a larger change we should defer until need is show.

NickEntin commented 8 years ago

I'm pretty sure we used them, or at the very least discussed some possible custom post processors where they would be useful. I believe it is valuable to be able to know when the post processor has finished processing when used with a data source. Perhaps we should just change the comment to specify that the signal is only automatically trigger when the post processor is run via a data source?

adam-zethraeus commented 8 years ago

right now the post processors have to fire this superfluous signal themselves.

let's change that.