Skip to content

Spike/idg modifications #36

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.

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

Draft
wants to merge 147 commits into
base: master
Choose a base branch
from

Conversation

idg10
Copy link

@idg10 idg10 commented May 26, 2023

First two chapters currently have more or less the structure and examples I want.

Copy link

@HowardvanRooijen HowardvanRooijen left a comment

Choose a reason for hiding this comment

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

Chapter 09 reviewed.

Copy link

@HowardvanRooijen HowardvanRooijen left a comment

Choose a reason for hiding this comment

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

Chapter 11 reviewed.


Let's follow exactly what happens here. First, assume that this code is just running normally and not in any unusual context—perhaps inside the `Main` entry point of a program. When this code calls `Subscribe` on the `IObservable<int>` returned by `SelectMany`, that will in turn will call `Subscribe` on the `IObservable<int>` returned by the first `Observable.Range`, which will in turn schedule a work item for the generation of the first value in the range (`1`).

Since we didn't pass a scheduler explicitly to `Range`, it will use its default choice, the `CurrentThreadScheduler`, and that will ask itself "Am I already in the middle of handling some work item on this thread?" In this case the answer will be no, so it will run the work item immediately (before returning from the `Schedule` call made by the `Range` operator). The `Range` operator will then produce its first value, calling `OnNext` on the `IObserver<int>` that the `SelectMany` operator provided when it subscribed to the range.

Choose a reason for hiding this comment

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

will be no, so it will
The so feels out of place

Copy link
Author

Choose a reason for hiding this comment

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

Really? The sentence makes no sense if I remove it. Something needs to signify that the remainder of the sentence is a consequence of the answer being no.

I've put the no in quotes, which I think makes its proximity to "so" look less odd.

This uses an extension method that Rx supplies for cases where you have no need for the `state` argument. In fact it's the wrong thing to use here, because this could end up passing the wrong scheduler. If you look at the `IScheduler` interface, you'll see that the work item callback's first argument is an `IScheduler`, and you're supposed to use that scheduler for any logically recursive work. Some schedulers (e.g., the `ImmediateScheduler`) supply a special type of scheduler specifically to avoid logical recursion turning into actual recursion that could end up going so deep that the application crashes with a stack overflow. This particular extension method isn't meant for these logically recursive scenarios, so we can't quite make it this simple. But perhaps we could avoid that weirdness of passing our own `this` argument:

```cs
var next = scheduler.Schedule<object?>(null, (innerScheduler, _) => LoopRec(innerScheduler));

Choose a reason for hiding this comment

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

So is the this manifested as the discard?

Copy link
Author

Choose a reason for hiding this comment

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

I don't understand the question. I'm not sure how to change the text to prevent a misunderstanding because I can't work out what you think is happening here.

The discarded parameter here is where the state gets passed back into our callback. In this case we've passed the Schedule method a state of null. So the thing being discarded here is null.

I've realised that one of the examples this section showed was a bit pointless, so I've removed it, and modified the discussion to fit, so I'm hoping that with that, it will be easier to follow what's going on.

Choose a reason for hiding this comment

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

But perhaps we could avoid that weirdness of passing our own this argument:

I don't see "our own this argument" - where is it in the code???

Copy link
Author

Choose a reason for hiding this comment

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

This is a reference to the preceding example, which looks like this:

var next = scheduler.Schedule(this, static (innerScheduler, @this) => @this.LoopRec(innerScheduler));

I've tweaked the text again to make it clear that I'm referring to the earlier example.

Copy link

@HowardvanRooijen HowardvanRooijen left a comment

Choose a reason for hiding this comment

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

Chapter 13 Reviewed

Copy link

@HowardvanRooijen HowardvanRooijen left a comment

Choose a reason for hiding this comment

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

Chapter 15 reviewed

Copy link

@HowardvanRooijen HowardvanRooijen left a comment

Choose a reason for hiding this comment

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

Chapter 16 reviewed

And some examples of some sources that might make good cold observables:

* the contents of a collection (such as is returned by the [`ToObservable` extension method for `IEnumerable<T>`](03_CreatingObservableSequences.md#from-ienumerablet))
* a fixed range of values, such as [`Observable.Range`](03_CreatingObservableSequences.md#observablerange) produces

Choose a reason for hiding this comment

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

03_CreatingObservableSequences.md#observablerange

Should be 03_CreatingObservableSequences.md#observable.range

Copy link
Author

Choose a reason for hiding this comment

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

Should it? VS code thinks it's right as it is. If I add the full stop as you suggest, the link is now broken inside VS Code.

Is this another "opinion is divided on the matter" markdownism?

Choose a reason for hiding this comment

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

It might be two conflicting approaches.

The Id generated by that heading is https://www.introtorx.com/chapters/creating-observable-sequences#observable.create

Choose a reason for hiding this comment

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


* the contents of a collection (such as is returned by the [`ToObservable` extension method for `IEnumerable<T>`](03_CreatingObservableSequences.md#from-ienumerablet))
* a fixed range of values, such as [`Observable.Range`](03_CreatingObservableSequences.md#observablerange) produces
* events generated based on an algorithm, such as [`Observable.Generate`](03_CreatingObservableSequences.md#observablegenerate) produces

Choose a reason for hiding this comment

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

03_CreatingObservableSequences.md#observablegenerate

Should be 03_CreatingObservableSequences.md#observable.generate

* a fixed range of values, such as [`Observable.Range`](03_CreatingObservableSequences.md#observablerange) produces
* events generated based on an algorithm, such as [`Observable.Generate`](03_CreatingObservableSequences.md#observablegenerate) produces
* a factory for an asynchronous operation, such as [`FromAsync`](03_CreatingObservableSequences.md#from-task) returns
* events produced by running conventional code such as a loop; you can create such sources with [`Observable.Create`](03_CreatingObservableSequences.md#observablecreate)

Choose a reason for hiding this comment

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

03_CreatingObservableSequences.md#observablecreate

Should be 03_CreatingObservableSequences.md#observable.create

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants