On Wednesday, 13 December 2017 at 03:15:11 UTC, IM wrote:
Thanks!
First thing.
Generally in D module names are kept lower-case.
To give an example your:
AbstractTaskRunner.d
module tasks.AbstractTaskRunner;
Would usually be:
abstracttaskrunner.d
module tasks.abstracttaskrunner;
Of course it's not necessary, but it's how it usually is in
idiomatic D.
The same goes for function names etc. they're usually kept
camelCase and not PascalCase like you have it. Again it's not
necessary, but that's idiomatic D.
A wild guess from me would be that you come from a C#/.NET
background OR you have worked heavily with the win api, am I
right?
Documentation can be done like this for multiline:
/**
* ...
* ...
* etc.
*/
Instead of:
/// ...
/// ...
/// etc.
This might just be a personal thing for me, but I always use ///
for single-line documentation only.
Also you don't need to define constructors for classes, if they
don't actually do something.
Ex. your SingleThreadTaskRunner class defines this:
~this() @safe {}
Integers are automatically initialized to int.init which is 0, so
the following is can be rewritten from: (Find in a unittest)
int value = 0;
to:
int value;
Also empty lambdas like:
() { ... }
can be written like:
{ ... }
Also you have a few Hungarian notations, which is often not used
in idiomatic D.
The key for an associative array __should__ always be treated
const internally, so this:
ITaskRunner[const Tid]
shouldn't be necessary and doing:
ITaskRunner[Tid] should work just fine.
If you have experienced otherwise, then I'd argue it's a bug and
you should report it.
Also statements like this:
immutable int currentNumber
are often preferred written like:
immutable(int) currentNumber
And getting too used to write:
immutable int currentNumber
might have you end up writing something like:
immutable int getAnImmutableNumber() { ... }
Which does not do what you would expect as it doesn't treat the
return value as immutable, but instead "this" as immutable.
To make the return type immutable you'd write:
immutable(int) getAnImmutableNumber() { ... }
I don't know if you know that already, but just figured I'd give
a heads up about that.
That was all I could see looking through your code real quick.
Take it with a grain of salt as some of the things might be
biased.
