Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add lightweight generics to RACChannel and RACChannelTerminal. #88

Merged
merged 6 commits into from
Apr 4, 2017
Merged

Add lightweight generics to RACChannel and RACChannelTerminal. #88

merged 6 commits into from
Apr 4, 2017

Conversation

erwald
Copy link
Member

@erwald erwald commented Apr 3, 2017

Both of them now have exactly one associated value (a covariant).

@erwald
Add lightweight generics to RACChannel and RACChannelTerminal.
cf5befc 
Both of them now have exactly one associated value (a covariant).
erichoracek
erichoracek reviewed Apr 3, 2017
View reviewed changes
ReactiveObjC/RACChannel.h Outdated
@@ -65,7 +65,7 @@ NS_ASSUME_NONNULL_BEGIN
/// terminals.
///
/// Do not instantiate this class directly. Create a RACChannel instead.
@interface RACChannelTerminal : RACSignal <RACSubscriber>
@interface RACChannelTerminal<__covariant ValueType> : RACSignal <RACSubscriber>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't a <ValueType> annotation required after RACSignal here, e.g.:

RACChannelTerminal<__covariant ValueType> : RACSignal<ValueType>`

ReactiveObjC/RACChannel.h Outdated
@@ -65,7 +65,7 @@ NS_ASSUME_NONNULL_BEGIN
/// terminals.
///
/// Do not instantiate this class directly. Create a RACChannel instead.
@interface RACChannelTerminal : RACSignal <RACSubscriber>
@interface RACChannelTerminal<__covariant ValueType> : RACSignal <RACSubscriber>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does sendNext: need to be redeclared here with ValueType?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you're right. Fixed here as for RACSubject.

ReactiveObjC/RACChannel.h Outdated
@@ -31,22 +31,22 @@ NS_ASSUME_NONNULL_BEGIN
/// on the `followingTerminal`, and received in the model from the
/// `leadingTerminal`. However, the initial value of the view is not received
/// from the `leadingTerminal` (only future changes).
@interface RACChannel : NSObject
@interface RACChannel<__covariant ValueType> : NSObject
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't RACChannel invariant since it is a sink and source of values?

ReactiveObjC/RACChannel.h Outdated
@@ -65,7 +65,7 @@ NS_ASSUME_NONNULL_BEGIN
/// terminals.
///
/// Do not instantiate this class directly. Create a RACChannel instead.
@interface RACChannelTerminal : RACSignal <RACSubscriber>
@interface RACChannelTerminal<__covariant ValueType> : RACSignal <RACSubscriber>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't RACChannelTerminal invariant since it is a sink and source of values?

ReactiveObjC/RACChannel.m Outdated
@@ -11,15 +11,15 @@
#import "RACReplaySubject.h"
#import "RACSignal+Operations.h"

@interface RACChannelTerminal ()
@interface RACChannelTerminal<__covariant ValueType> ()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't RACChannelTerminal invariant since it is a sink and source of values?

ReactiveObjC/UIStepper+RACSignalSupport.h Outdated
@@ -8,7 +8,7 @@

#import <UIKit/UIKit.h>

@class RACChannelTerminal;
@class RACChannelTerminal<__covariant ValueType>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't RACChannelTerminal invariant since it is a sink and source of values?

ReactiveObjC/UISwitch+RACSignalSupport.h Outdated
@@ -8,7 +8,7 @@

#import <UIKit/UIKit.h>

@class RACChannelTerminal;
@class RACChannelTerminal<__covariant ValueType>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't RACChannelTerminal invariant since it is a sink and source of values?

ReactiveObjC/UITextField+RACSignalSupport.h Outdated
@@ -8,7 +8,7 @@

#import <UIKit/UIKit.h>

@class RACChannelTerminal;
@class RACChannelTerminal<__covariant ValueType>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't RACChannelTerminal invariant since it is a sink and source of values?

ReactiveObjC/UIControl+RACSignalSupportPrivate.h Outdated
@@ -27,3 +29,5 @@
- (RACChannelTerminal *)rac_channelForControlEvents:(UIControlEvents)controlEvents key:(NSString *)key nilValue:(id)nilValue;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't nilValue nullable? e.g. - (RACChannelTerminal *)rac_newSelectedSegmentIndexChannelWithNilValue:(nullable NSNumber *)nilValue; calls through to this with a nullable value

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Fixed and added a clarifying sentence to the method documentation.

ReactiveObjC/RACChannel.h Outdated
@@ -9,7 +9,7 @@
#import "RACSignal.h"
#import "RACSubscriber.h"

@class RACChannelTerminal;
@class RACChannelTerminal<__covariant ValueType>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't RACChannelTerminal invariant since it is a sink and source of values?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, of course! Fixed, here and elsewhere.

Erich Grunewald added 5 commits April 4, 2017 10:49
Make RACChannel, RACChannelTerminal and RACKVOChannel have invariant …
3c69e56 
…generic type.

Because they are both a source and a sink, they oughn’t to have
covariant generics.
Redeclare -sendNext: in order to have it take the associated type.
c7f723b 
For RACChannelTerminal.
Make private method's nilValue argument a nullable.
04c44a1 
It's possible that one wants to have nil values actually be nil,
so annotate the argument accordingly.
Make comment above rac_channelForControlEvents:key:nilValue: method d…
8ab85cb 
…ocumentation comment.
Make comments above RACChannelTerminal properties documentation comme…
2c4622f 
…nts.
@erwald
Copy link
Member Author

erwald commented Apr 4, 2017

@erichoracek Thanks for the comments; I've addressed all of them, so feel free to have another look whenever you have time.

erichoracek
erichoracek approved these changes Apr 4, 2017
View reviewed changes
Copy link
Contributor

@erichoracek erichoracek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! 🎉

@erichoracek erichoracek merged commit 0afab97 into ReactiveCocoa:master Apr 4, 2017
erichoracek added a commit to erichoracek/ReactiveObjC that referenced this pull request Apr 7, 2017
@erichoracek
Increase version to 2.2.0
ba66a42 
Includes ReactiveCocoa#75, ReactiveCocoa#80, ReactiveCocoa#81, ReactiveCocoa#82, ReactiveCocoa#83, ReactiveCocoa#85, ReactiveCocoa#87, and ReactiveCocoa#88.

As far as I can tell, this should have no breaking changes.
@erichoracek erichoracek mentioned this pull request Apr 7, 2017
Closed
erichoracek added a commit to erichoracek/ReactiveObjC that referenced this pull request Apr 7, 2017
@erichoracek
Increase version to 3.0.0
953a0cf 
Includes ReactiveCocoa#75, ReactiveCocoa#80, ReactiveCocoa#81, ReactiveCocoa#82, ReactiveCocoa#83, ReactiveCocoa#85, ReactiveCocoa#87, and ReactiveCocoa#88.

While this has no breaking changes in Obj-C, it will likely introduce breaking changes in Swift.
@erichoracek erichoracek mentioned this pull request Apr 7, 2017
Merged
stuartofmine pushed a commit to stuartofmine/ReactiveObjC_2023 that referenced this pull request Oct 18, 2023
@erichoracek
Merge pull request ReactiveCocoa#88 from erwald/racchannel-generics
a3d6e1f 
Add lightweight generics to RACChannel and RACChannelTerminal.
stuartofmine pushed a commit to stuartofmine/ReactiveObjC_2023 that referenced this pull request Oct 18, 2023
@erichoracek
Increase version to 3.0.0
2195f16 
Includes ReactiveCocoa#75, ReactiveCocoa#80, ReactiveCocoa#81, ReactiveCocoa#82, ReactiveCocoa#83, ReactiveCocoa#85, ReactiveCocoa#87, and ReactiveCocoa#88.

While this has no breaking changes in Obj-C, it will likely introduce breaking changes in Swift.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erichoracek erichoracek erichoracek approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@erwald @erichoracek