Is it possible to request a code review?

rjframe dlang at ryanjframe.com
Thu Dec 14 12:51:07 UTC 2017


On Wed, 13 Dec 2017 03:15:11 +0000, IM wrote:

> Being new to D, I probably made many mistakes, or did things in a way
> where there's a more optimum one to do in D. I'm eager to know how to
> improve, and would like to know if there is any experienced D developer
> who is thankfully willing to take a look at it and give me feedback?
> 
> Thanks!

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.


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.

`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.

I only skimmed other reviews, so this may have been mentioned; in 
`TaskQueue.d`, a private function is misspelled:
"AppednTaskThreadUnsafe" -> "AppendTaskThreadUnsafe".

--Ryan


[1]: http://code.dlang.org/packages/unit-threaded
[2]: http://code.dlang.org/packages/tested


More information about the Digitalmars-d mailing list