Skip to content

Conversation

@knocte
Copy link

@knocte knocte commented Jan 19, 2016

Two improvements here:

  1. It's better to show the slowest first.
  2. It's better to not 'open Nessos.Streams', and use the
    namespace explicitly.
    It took me a minute to wrap my head around this code,
    so I think with these 2 improvements, other will understand
    it a bit better the first time they read it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is using the full namespace better?

Copy link
Author

Choose a reason for hiding this comment

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

Because one can distinguish in a quicker way that it's using the library in this block.

Copy link
Author

Choose a reason for hiding this comment

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

(i.e. had no idea about ParStream class until I saw the sources...)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's better to do it by adding a comment

@knocte
Copy link
Author

knocte commented Jan 22, 2016

I've now used type aliasing instead, should be clearer and not so repetitive.

Two improvements here:
1. It's better to show the slowest first.
2. Rather use type aliasing instead of namespace opening
(before, the connection between `open Nessos.Streams`
and Stream|ParStream was not so clear).

It took me a minute to wrap my head around this code,
so I think with these 2 improvements, others will understand
it a bit better the first time they read it.
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.

2 participants