On Thursday, 14 December 2017 at 12:51:07 UTC, rjframe wrote:
On Wed, 13 Dec 2017 03:15:11 +0000, IM wrote:
[...]
I like this API design.
I would separate each unit test to its own `unittest` block
(you have three tests in a single block in
`SingleThreadTaskRunner.d`); it doesn't really matter with the
integrated runner, but if you later use a runner like
unit-threaded[1] or tested[2] each unit test block becomes its
own independent test, and they will all be run, even if one or
more fails.
Interesting. I haven't decided which tester runner to use yet,
because the built in one is minimal and is getting me going. But
thanks for the pointers. `tested` seems interesting because it's
minimal and I like minimal things, however it seems it hasn't
been touched for 2 years.
If `TaskQueue.Pop` returned the Task rather than used an out
parameter,
this (in SingleThreadTaskRunner.RunnerLoop):
```
while (true) {
Task front;
mQueue.Pop(front);
if (!front)
return;
front();
}
```
could become:
```
while (true) {
auto front = mQueue.Pop();
front ? front() : return;
}
```
Though that's probably debatable as to which is more readable.
I would
avoid `out` parameters on void functions; I would at least
return a `bool`
and test that instead (which is what `TryPop` does):
```
while (true) {
Task front;
if (!mQueue.Pop(front)) {
return;
}
front();
```
I would probably switch the if block to the affirmative case,
but that's personal preference.
I like that. I wouldn't switch it to affirmative because I like
the early return and not having to add an `else`.
`Pop` could call `TryPop` to eliminate code duplication. The
same is true for other Try functions.
I'll ignore the style since others have discussed it and it's
not what you're looking for, but this does look like C#. Style
actually is somewhat- important; if I use your library in a
project, it would be nice if API calls look like they belong in
my code. That's why everybody recommends at least following the
Phobos naming guidelines for published libraries.
That's a fair point, and I agree. I will consider switching to
the Phobos
guidelines at some point.
I only skimmed other reviews, so this may have been mentioned;
in `TaskQueue.d`, a private function is misspelled:
"AppednTaskThreadUnsafe" -> "AppendTaskThreadUnsafe".
Oops, thanks!
--Ryan
[1]: http://code.dlang.org/packages/unit-threaded
[2]: http://code.dlang.org/packages/tested
