Improve wording in Riddle 6 and add Riddle 102 #20
Conversation
|
|
||
| /** Solution [Riddle6Solution] */ | ||
| class Riddle6Test { | ||
| @get:Rule val rxRule = RxRule() |
There was a problem hiding this comment.
I don't use RxRule because I couldn't figure out how to check that the observables were subscribed concurrently but by two different schedule* calls. This was necessary to disallow subscribing once for the entire zip
src/test/kotlin/com/vanniktech/rxriddles/solutions/Riddle6Solution.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
How about we add to the description of Riddle that it can be assumed that each Single will run on it's own thread when passed. That way we only have to use one operator.
In the tests, we can create Observables and apply the schedulers.
I prefer having a Single line solution for Riddle6.
That's of course up to you, but I don't think it's a good idea. I understand that one-operator solutions are preferable, but I still believe the entire difficulty in doing work in parallel requires understanding that inner observables need to be subscribed on a different thread. Doing it for someone is not very educational. Would it be ok if I modify this riddle as you say to only add the comment, but add Riddle 38 (or 102?) which is essentially what I wrote here? Would such slightly more complicated riddle be ok as one of the later ones? |
|
Yes. Let's do a one-liner for this riddle. And then the more complicated one with adding the scheduler yourself can be 102. |
|
@vanniktech I've updated the PR, now Riddle6 has only a comment added, and there's new Riddle102 that requires more operators. @mateuszkwiecinski I've also updated the test to require |
|
|
||
| object Riddle102Solution { | ||
| fun solve(first: Single<Int>, second: Single<Int>) | ||
| = Single.zip( |
There was a problem hiding this comment.
I think formatting is different here from the other solutions. Can you double check?
There was a problem hiding this comment.
Yep, I was looking at riddles < 100 and they have = in new lines, while 100+ have = just after function declaration. I adjusted it to have = Single.zip( in previous line and parameters in others. Let me know if you meant only = in previous line, and Single.zip in the next one, to have parameters indent themselves more.
As a side note, it'd be useful if you checked in intellij's code style settings file. From what I see it's somewhat similar to Square's codestyle? But there were differences anyway
There was a problem hiding this comment.
Yeah, it's similar to Square's codestyle but has some differences. I typically don't ues the formatting options though.
| RxJavaPlugins.setIoSchedulerHandler { testScheduler } | ||
| RxJavaPlugins.setSingleSchedulerHandler { testScheduler } | ||
| RxJavaPlugins.setNewThreadSchedulerHandler { testScheduler } | ||
| override fun apply(base: Statement, description: Description): Statement = object : Statement() { |
There was a problem hiding this comment.
Can we you revert the object : Statement? Or is this necessary?
There was a problem hiding this comment.
We need it, since apply returns Statement. We can't return base anymore, because now we call reset() after the test.
… to a separate thread
| @@ -5,6 +5,8 @@ import io.reactivex.Single | |||
| object Riddle6 { | |||
| /** | |||
| * Execute both [first] and [second] Single's in parallel and provide both results as a pair. | |||
| * Assume both [first] and [second] will execute on a different thread already. | |||
| * This is a slightly simpler version of [Riddle102], where no schedulers are applied by default. | |||
vanniktech
changed the title
My suggestion to solving #4
I tried to keep the original code style, but it's not stored in the project, so I might've missed something