Is it possible to request a code review?

bauss jj_1337 at live.dk
Wed Dec 13 06:14:04 UTC 2017


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.


More information about the Digitalmars-d mailing list