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