www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - The best coding advice that was ever given to me

reply Mathias LANG <geod24 gmail.com> writes:
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/3292bbc89c4da4258c1b05a7ae6d4c9570d
43d1?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".
Dec 29 2020
next sibling parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Wednesday, 30 December 2020 at 03:06:49 UTC, Mathias LANG 
wrote:

 In DMD's defense, the adage used to be "only have one exit 
 point per function".


that was prolly because it was before the age of scope guards...
Dec 29 2020
parent Basile B. <b2.temp gmx.com> writes:
On Wednesday, 30 December 2020 at 03:08:14 UTC, Adam D. Ruppe 
wrote:

 On Wednesday, 30 December 2020 at 03:06:49 UTC, Mathias LANG 
 wrote:

 In DMD's defense, the adage used to be "only have one exit 
 point per function".


 that was prolly because it was before the age of scope guards...


There was also the problem of DMD inliner that didn't like much 
early returns; although the situation toward this problem migh be 
better nowadays.
Dec 29 2020
prev sibling parent Walter Bright <newshound2 digitalmars.com> writes:
On 12/29/2020 7:06 PM, Mathias LANG wrote:

 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
 [...]
 In DMD's defense, the adage used to be "only have one exit point per function".


Recall also that DMD was written in C-with-classes, which does not have nested 
functions, so goto was the best alternative.
Dec 29 2020