Allow NSObject.reactive.signal to also intercept and return the 'return' value of the executed selector. #3557
Conversation
…elector) NSObject.reactive.signal(for selector: Selector, includeReturnValue: Bool = false) Will add a return value to the end of the NSArray. If the method returns Void, than the flag is ignored and no value is appended to the NSArray.
mishagray
force-pushed
the
reactive_signal_include_return_vals
branch
from
03c17dd
to
f37617d
Compare
|
It looks great with a quick skim. I will take a second look ASAP. In the meantime, could you please clean up the indentation, the excess blank lines & break the long lines? The GitHub shows quite a few nits. |
…agray/ReactiveCocoa into reactive_signal_include_return_vals
| /// - returns: A signal that sends an array of bridged arguments. | ||
| public func signal(for selector: Selector) -> Signal<[Any?], NoError> { | ||
| return base.intercept(selector).map(unpackInvocation) | ||
| public func signal(for selector: Selector, includeReturnValue: Bool = false) -> Signal<[Any?], NoError> { |
There was a problem hiding this comment.
This is API-compatible but binary-incompatible change. So the original API should be kept as is and the new API should be added as a separete one.
| /// - selector: The selector to observe. | ||
| /// - includeReturnValue: the array will include the value returned by the invocation. Ignored if the selector returns Void. | ||
| /// - returns: A signal that sends an array of bridged arguments. | ||
| public func signal(for selector: Selector, includeReturnValue: Bool) -> Signal<[Any?], NoError> { |
There was a problem hiding this comment.
This seems to me the wrong way to do this. Instead, it would be better to have a struct that tied together the parameters and return value.
struct Invocation {
var parameters: [Any?]
var returnValue: Any?
}|
|
||
| invocation.copyReturnValue(to: buffer) | ||
| value = NSValue(bytes: buffer, objCType: retRawEncoding) | ||
| } |
There was a problem hiding this comment.
There's code duplication here between this and the parameters. I think they could be combined so that all this logic doesn't need to be repeated.
| case is CInt.Type: | ||
| return #selector(InterceptedObject.testReturnValuesI(arg:)) | ||
| case is CLong.Type: | ||
| return #selector(InterceptedObject.testReturnValuesL(arg:)) |
There was a problem hiding this comment.
I think this logic unnecessarily complicates the test. It would be better to pass in the Selector directly if possible.
Added the
includeReturnValueto reactive.signal(for selector: Selector)NSObject.reactive.signal(for selector: Selector, includeReturnValue: Bool = false)
Will add a return value to the end of the array, for each value of the Signal.
If the selector does not return a value, than the flag is ignored and no value is appended to the array.
Checklist