The best coding advice that was ever given to me
Mathias LANG
geod24 at gmail.com
Wed Dec 30 03:06:49 UTC 2020
Could be summarized in two words: "early returns".
This advice was given (indirectly) to me by @andralex while
reviewing one of Walter's PR, and if I have to pick a single
thing that made my code and understanding of DMD much easier,
there's simply no other contender.
This advice was credited by Andrei to Code complete (edition 2),
chapter 17.1 if you own a copy. DMD is full of examples of why
this is a good thing. Here's the one I am currently working on:
https://github.com/dlang/dmd/blob/0385214d54aac8a4a31472de8ff41dcffc891641/src/dmd/dcast.d#L2724-L3403
You can see that this function, `typeMerge`, is 679 lines long.
It is invoked when types needs to be merged (duh), for example
with the ternary operator (auto res = a ? b : c) or other binary
operation (auto res = a + b).
I think I can safely say that looking at this, even for seasoned
contributors, is a bit intimidating. Lots of things can go wrong
while touching this function. Changing an innocent line at the
beginning might break code in unforeseeable ways, as it's a
forest of `goto` and `if`.
There's two main transformations which help to make sense of this:
- First one has to realize that the branches are *mutually
exclusive*; If you pick any branches, you will see that they
either exit their scope with a `return` or a `goto`, such as
[here](https://github.com/dlang/dmd/blob/0385214d54aac8a4a31472de8ff41dcffc891641/src/dmd/dcast.d#L3143), or will fall back to the [return at the end](https://github.com/dlang/dmd/blob/0385214d54aac8a4a31472de8ff41dcffc891641/src/dmd/dcast.d#L3402), such as [here](https://github.com/dlang/dmd/blob/0385214d54aac8a4a31472de8ff41dcffc891641/src/dmd/dcast.d#L3253-L3259).
- Second, whenever you see a `goto`, there's a high chance this
can be better expressed with a nested function.
Regarding the first point, it means that the following code:
```
void cmp (float a, float b)
{
int result;
if (a == b)
result = 0;
else if (a < b)
result = -42;
else if (a > b)
result = 42;
else
result = 0;
return result;
}
```
Can be expressed as:
```
void cmp (float a, float b)
{
if (a == b)
return 0;
if (a < b)
return -42;
if (a > b)
return 42;
return 0;
}
```
While the advice might sound obvious when looking at this toy
example, its value really become apparent at scale, for a
function such as `typeMerge`.
And regarding the `goto` => nested function transformation,
contributors have been doing this for a few years with great
results. E.g. in `typeMerge`:
https://github.com/dlang/dmd/commit/5b3a28416610535a4d415e0f27323ed64ea3349f
Other examples of said transformations:
-
https://github.com/dlang/dmd/commit/c9e649ae6aebfba3a6527df3929221628be142a9
-
https://github.com/dlang/dmd/pull/10399/commits/3292bbc89c4da4258c1b05a7ae6d4c9570dc43d1?diff=split&w=1 (Note that this removes one level of indentation, turn off `w=1` for an unreadable diff)
-
https://github.com/dlang/dmd/commit/4601b547a9fde3a03bf38cd2fa13e48670018e0d
In DMD's defense, the adage used to be "only have one exit point
per function".
More information about the Digitalmars-d
mailing list