Move more methods into <RACStream> #135

jspahrsummers · 2012-11-26T00:11:04Z

No description provided.

jspahrsummers · 2012-11-26T00:43:12Z

I mostly wanted to open this early because the takeWhileBlock: and takeUntilBlock: tests are failing for <RACSignal> (but not RACSequence). The take isn't properly being terminated early, so it ends up more like a filter.

With some debug logging added to bind::

Test Case '-[RACSignalSpec _RACStream__taking_with_a_predicate_should_take_until_a_predicate_is_true]' started.
  <RACStream> taking with a predicate should take until a predicate is true
2012-11-25 16:42:15.524 otest-x86_64[68214:707] <RACSignal: 0x7fafb3a03020> name: (null) got signal <RACSignal: 0x7fafb3a04690> name: (null) from bound value 0
2012-11-25 16:42:15.524 otest-x86_64[68214:707] <RACSignal: 0x7fafb3a03020> name: (null) got signal <RACSignal: 0x7fafb0e454b0> name: (null) from bound value 1
2012-11-25 16:42:15.525 otest-x86_64[68214:707] <RACSignal: 0x7fafb3a03020> name: (null) got signal <RACSignal: 0x7fafb1d01b90> name: (null) from bound value 2
2012-11-25 16:42:15.525 otest-x86_64[68214:707] <RACSignal: 0x7fafb3a03020> name: (null) got signal (null) from bound value 3
2012-11-25 16:42:15.526 otest-x86_64[68214:707] <RACSignal: 0x7fafb3a03020> name: (null) disposing (null) after bound value 3
2012-11-25 16:42:15.526 otest-x86_64[68214:707] <RACSignal: 0x7fafb3a03020> name: (null) got signal <RACSignal: 0x7fafb1b486d0> name: (null) from bound value 0
2012-11-25 16:42:15.526 otest-x86_64[68214:707] <RACSignal: 0x7fafb3a03020> name: (null) got signal <RACSignal: 0x7fafb1b44ec0> name: (null) from bound value 2
2012-11-25 16:42:15.527 otest-x86_64[68214:707] <RACSignal: 0x7fafb3a03020> name: (null) got signal (null) from bound value 4
2012-11-25 16:42:15.527 otest-x86_64[68214:707] <RACSignal: 0x7fafb3a03020> name: (null) disposing (null) after bound value 4
2012-11-25 16:42:15.528 otest-x86_64[68214:707] <RACSignal: 0x7fafb3a03020> name: (null) completed
2012-11-25 16:42:15.528 otest-x86_64[68214:707] ReactiveCocoa/ReactiveCocoaFramework/ReactiveCocoaTests/RACSignalSpec.m:87 expected: (0, 1, 2), got: (0, 1, 2, 0, 2)
ReactiveCocoa/ReactiveCocoaFramework/ReactiveCocoaTests/RACSignalSpec.m:87: error: -[RACSignalSpec _RACStream__taking_with_a_predicate_should_take_until_a_predicate_is_true] : expected: (0, 1, 2), got: (0, 1, 2, 0, 2)
Test Case '-[RACSignalSpec _RACStream__taking_with_a_predicate_should_take_until_a_predicate_is_true]' failed (0.005 seconds).

The failures here look like another manifestation of #94, but for a finite signal (somehow).

Coneko · 2012-11-26T01:32:38Z

Yep, it's definitely #94.
The signal in the test is created by calling the primitives +empty, +return: and -concat:. The first two primitives don't give the subscription a chance to dispose itself before sending nexts and completed, the latter doesn't exhibit the behavior per se, but propagates it.

jspahrsummers · 2012-11-26T01:56:51Z

@Coneko Thanks for the pointer.

I've added a workaround so that -bind: doesn't send more values than it should, but it doesn't address actual termination of the subscription.

jspahrsummers · 2012-11-26T02:38:54Z

This should be ready for review and merging now (contingent upon #134).

There are probably still a few other methods that could go into <RACStream>, but this is sufficiently disruptive for now.

The resulting combined logic should be easier to follow and fix.

Conflicts: ReactiveCocoaFramework/ReactiveCocoa/RACSignal.m ReactiveCocoaFramework/ReactiveCocoaTests/RACStreamExamples.m

joshaber · 2012-11-28T20:53:58Z

ReactiveCocoaFramework/ReactiveCocoa/RACStream.h

+// as determined by `==` or `isEqual:`.
+//
+// Returns a stream of the receiver's values with sequential repeats removed.
+- (instancetype)skipRepeats;


Is this actually useful?

This is just distinctUntilChanged generalized. I don't see why we'd want to keep it confined to <RACSignal>.

Right, I'm just not sure it's actually useful for sequences. A method to get a distinct sequence, sure, but this seems minimally useful.

Seems to be a pretty common question for NSArray, at least.

Right, so a method that removes all duplicates would be useful but this "Returns a stream of the receiver's values with sequential repeats removed."

Ah, quite right. I misinterpreted that search.

I'll revert this change.

This reverts commit 6104c9c. Conflicts: ReactiveCocoaFramework/ReactiveCocoa/RACStream.m

joshaber · 2012-11-28T22:05:02Z

ReactiveCocoaFramework/ReactiveCocoa/RACStream.h

+// Returns a stream which sends a RACTuple for each value in the receiver, where
+// the first object in the tuple is the value from the receiver, and the second
+// object in the tuple is `weakObject`.
+- (instancetype)injectObjectWeakly:(__weak id)weakObject;


I'd actually been meaning to remove this method entirely. Any objections to doing that instead?

Why's that?

I know we don't use it, but I could see the strong/weak dance being abhorrent to some people.

I think it leads to more awkward code than the strong/weak dance does.

joshaber · 2012-11-28T22:06:35Z

Other than the 1 comment about removing -injectObjectWeakly:, this looks ✨ to me.

jspahrsummers · 2012-11-28T22:29:46Z

📦

joshaber · 2012-11-28T22:33:49Z

🍢

Move more methods into <RACStream>

jspahrsummers added 6 commits November 25, 2012 15:05

Moved -mapReplace: into <RACStream>

b3f439a

Moved -injectObjectWeakly: into <RACStream>

0ae7ce1

Moved -scanWithStart:combine: into <RACStream>

db101cc

Moved -takeUntilBlock: and -takeWhileBlock: into <RACStream>

5af79e5

Additional termination testing for -bind: on infinite streams

6e3c0ee

Manually kill the disposable when -bind: terminates

dda82ae

Workaround for #94 specifically for the case of -bind:

d24b995

jspahrsummers added 3 commits November 25, 2012 17:59

Document the workaround

a212de4

Moved -skipWhileBlock: and -skipUntilBlock: into <RACStream>

8897a75

Moved -distinctUntilChanged into <RACStream> as -skipRepeats

6104c9c

jspahrsummers added 2 commits November 26, 2012 07:14

Merge branch 'master' into more-racstream

f3c3acc

Merge branch 'master' into more-racstream

6c82f0d

jspahrsummers closed this Nov 26, 2012

Implement -[RACSignal bind:] from scratch, without -flatten:

92a6850

The resulting combined logic should be easier to follow and fix.

jspahrsummers reopened this Nov 27, 2012

jspahrsummers added 7 commits November 27, 2012 10:29

Document expected -bind: semantics

6c4f9a9

Remove unnecessary __block qualifier

e0e656c

Added some additional memory management unit tests

3d01525

Add a memory management test to RACPropertySignalExamples

adf7b70

Merge branch 'bind-fixes' into more-racstream

741670b

Conflicts: ReactiveCocoaFramework/ReactiveCocoa/RACSignal.m ReactiveCocoaFramework/ReactiveCocoaTests/RACStreamExamples.m

Update added methods to use the new -bind: interface

4502950

Merge branch 'bind-fixes' into more-racstream

c90cc49

joshaber reviewed Nov 28, 2012
View reviewed changes

jspahrsummers added 2 commits November 28, 2012 13:12

Revert "Moved -distinctUntilChanged into <RACStream> as -skipRepeats"

44c05e4

This reverts commit 6104c9c. Conflicts: ReactiveCocoaFramework/ReactiveCocoa/RACStream.m

Explicitly compare against nil

439a4bf

joshaber reviewed Nov 28, 2012
View reviewed changes

jspahrsummers added 3 commits November 28, 2012 14:15

Merge branch 'master' into more-racstream

7691e10

Just remove -injectObjectWeakly:

8a57788

Refactor GHAPIDemo to not use -injectObjectWeakly:

48a4c53

ghost assigned joshaber Nov 28, 2012

joshaber added a commit that referenced this pull request Nov 28, 2012

Merge pull request #135 from github/more-racstream

13b1fa0

Move more methods into <RACStream>

joshaber merged commit 13b1fa0 into master Nov 28, 2012

jifang mentioned this pull request Feb 4, 2016

Deadlock of NSLock on same(?) thread #2707

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move more methods into <RACStream> #135

Move more methods into <RACStream> #135

jspahrsummers commented Nov 26, 2012

jspahrsummers commented Nov 26, 2012

Coneko commented Nov 26, 2012

jspahrsummers commented Nov 26, 2012

jspahrsummers commented Nov 26, 2012

joshaber Nov 28, 2012

jspahrsummers Nov 28, 2012

joshaber Nov 28, 2012

jspahrsummers Nov 28, 2012

joshaber Nov 28, 2012

jspahrsummers Nov 28, 2012

joshaber Nov 28, 2012

jspahrsummers Nov 28, 2012

joshaber Nov 28, 2012

jspahrsummers Nov 28, 2012

joshaber commented Nov 28, 2012

jspahrsummers commented Nov 28, 2012

joshaber commented Nov 28, 2012

Move more methods into <RACStream> #135

Move more methods into <RACStream> #135

Conversation

jspahrsummers commented Nov 26, 2012

jspahrsummers commented Nov 26, 2012

Coneko commented Nov 26, 2012

jspahrsummers commented Nov 26, 2012

jspahrsummers commented Nov 26, 2012

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshaber commented Nov 28, 2012

jspahrsummers commented Nov 28, 2012

joshaber commented Nov 28, 2012