Code of coding 2: Documenting changes

Few days ago, when I wrote about coding, I didn’t have a slightest idea that at the same time, at the completely opposite part of the globe, Dave was blogging almost about the same thing. It is interesting to know that I am not the only one out there actually worying about code, and how it looks like.

The most common thing I used to hear when I asked bad coders about their code was: “It works, why shoud I care.”

When Dave wrote about what programmer intended, I immediately remembered one of the real gems of coding, which classify to both whaddaf* and huh? categories simultaneously. So, here’s the deal, whoever comments on this first with correct explanation of what this part of code really does, gets bragging rights and full credit in my next post:

GLEntry.SETCURRENTKEY("G/L Account No.");
IF GLEntry.FIND('-') THEN REPEAT
  GLEntryTmp := GLEntry;
  GLEntryTmp.INSERT;
  GLEntry.SETRANGE("G/L Account No.",GLEntry."G/L Account No.");
  GLEntry.FIND('+');
  GLEntry.SETRANGE("G/L Account No.");
UNTIL GLEntry.NEXT = 0;

This is not too complicated, but might get you off guard at first. Anyway, this is one of the best examples of if it was hard to write it should be hard to understand approach I’ve seen in NAV. But this wasn’t my topic for today. My topic for today is something I already announced last time, my Rule No. 3: Preserve the environment.

Although I had quite a crazy idea of writing a post about writing environmentally friendly code, I’ll pass, at least for now. Environment I am concerned about here is the environment around the code which you are modifying. There is hardly any implementation of Microsoft Dynamics NAV (or any other ERP) which doesn’t require you to stick your knife deep into its heart, rip apart the standard code and insert your own, for better or worse. There is absolutely nothing wrong with this, this can’t be avoided, because there is hardly any customer who can completely fit into the out-of-the box system, and you always have to tailor a bit here and there.

When we talk about modifications, primary concern about them is the one easily forgotten: upgradeability. In plain English, it is how easy it will be to eventually upgrade whatever you have done to a future version of the system. The problem of upgrade is not a trivial one, it is not the matter of simply determining the delta between the old version and the new version, and then applying it against the new one. It’s far more complicated. It is actually determining the delta between the old base version and the new base version, then the delta between the old customized version and the new base version, and then the delta between these two deltas. This is something that computer can do fairly well, allright, but add to this equation the meaning of customization and the developers intentions, we introduce the immaginary component and truly make this a full-blooded complex problem.

There are three types of modifications:

  1. Additions: Whenever you add something new which wasn’t already there, it is an addition. It can be adding a line or a block of code to a codeunit, adding a completely new function to a codeunit, adding a field to a table, you name it. These are the easiest forms of modifications, because they both usually put very low pressure on upgradeability, and require a true fool to really mess something up. Additions never conflict with anything, they stand alone, have no dependencies upon themselves, and can normally just be copied from the old version to the new one without making a rocket science out of it.
  2. Alterations: When you modify something, either quantitatively by introducing new logic to existing one, or qualitatively by exchanging existing logic for a new one, it is an alteration. Typically we talk about altering existing lines of code by replacing them with something new, either partially or fully, by simply extending them with some new conditions, branching and similar, or stripping them off the same. When you extend the length of a field, or add new options to existing ones on an option field, these are also alterations. Now these are ugly sonsofabitches. Alterations cause most of the headaches during upgrade processes, because if done improperly, they may conceal both original logic and programmers intentions, and cause full scale disasters to upgraded versions. We are going to focus on these today, so stick them in your mind for a while.
  3. Deletions: When you delete something that was there before, but is no more, it is deletion. If you delete a whole lines or blocks of code or functions, or fields from tables, whatever, this is deletion. They are almost as easy as additions, having probably the same low pressure on upgrade, but potentially having detrimental effect on current version. With deletion, some existing logic goes away, and you must be extra careful (remember rule no. 1) not to delete something which has something else depending on it, God forbid something critical, otherwise you better start revising your resume.

So, what do we do now with these three? We document them. As much as coding should be self documenting, a concept Dave wrote about in the post I mentioned before, there is hardly a point in writing something like this:

// Assigning new quantity
SalesLine.VALIDATE(Quantity, qty);
// Writing data back to the database
SalesLine.MODIFY(TRUE);

What I am talking about is this:

// 2008-02-15 - VB - CR-390 >
SalesLine.VALIDATE(Quantity, qty);
SalesLine.MODIFY(TRUE);
// 2008-02-15 - VB - CR-390 <

Now this is something different. Documenting in this case was explaining not what have been done in terms of business logic (which if done correctly, should be self-explanatory), but what have been done in terms of meta information. Or to put it simpler, it is about what we have done to the code, not to the logic. Other than that, we may also describe why we have done so. Our example above keeps an awful lot of information: it says that one VB has on February 15, 2008 added some part of code because change request ID 390 instructed them to do so, and it gives information where exactly does this addition start, and where it ends.

Of course, you don’t have to use the convention above, it is just an example of a descriptive one. You do whatever works best for you. However, there is a recommendation I’m going to make: clearly mark the beginning and the end of your modifications, even if you just modify a single line of code. So, please don’t do this:

SalesLine.MODIFY(TRUE); // 2008-02-15 - VB - CR-390

Rather, do this:

// 2008-02-15 - VB - CR-390 >
SalesLine.MODIFY(TRUE);
// 2008-02-15 - VB - CR-390 <

Why do I recommend this? For the sake of clarity. When you mark your line with a trailing comment, you make it less prominent, and one can easily skip over it when looking at code, one might even think it is a standard line of code, especially if you add a line right in a middle of other lines of code, and especially when they are unusually long, or extremely similar to each other. The second example, however, stands out prominently and you can hardly ever miss it.

This above was about documenting additions, but what do we do with alterations? Suppose we want to take away the LastOperation condition from this piece of code:

IF LastOperation AND ("Output Quantity" <> 0) THEN BEGIN
  CheckItemTracking;

Now, how do we do that? The only way I find correct is this:

// 2008-02-15 - VB - CR-390 >
// IF LastOperation AND ("Output Quantity" <> 0) THEN BEGIN

IF ("Output Quantity" <> 0) THEN BEGIN

// 2008-02-15 - VB - CR-390 <<
  CheckItemTracking;

What do we accomplish with this? Well, for starters, we provide exactly the same amount of detail about our modification as we did while documenting an addition. Plus, we also preserve the original logic, so that it can be compared with new one, or reversed altogether if necessary. Also, it makes it much easier to understand what exactly did we do.

Last, but not least, the deletions. For Pete’s sake, don’t just delete the line, don’t do this:

(this line intentionally left blank)

Be nice and do this: 

// 2008-02-15 - VB - CR-390 >
// SalesLine.VALIDATE(Quantity, qty);
// 2008-02-15 - VB - CR-390 <

 The effect to the execution is that this line of code is not going to be executed, as if it was deleted. However, as with documenting modification, here you preserved the environment too, and made it perfectly clear what did the code look like before you came and decided that something stinks.

The benefit we get for all this pain of extra coding is that our code is understandable on a completely new layer, a meta-layer. When anyone looks at the code, they can immediately tell what kind of modification has been made, and when they need to decide on how to proceed, they have whole original environment preserved for them, which makes for a good decision-making tool. These documentations are especially helpful when upgrading to a new version, they can save you inumerable hours just like that.

Another nice thing, but not paramount, is to list all of the modifications you ever did to the object in the Documentation trigger. As long as you did all of the in-place documentations right, you may live quite as well without the documentation trigger, but it helps a lot. The only reason I am allowing that documentation trigger be left blank is that it doesn’t contain any logic, and every changes to logic have already been documented in-place in the code, and also, if you are doing your project documentation right, you should have all modifications listed elsewhere with much more detail than can usefully squeeze into this trigger.

I’ve been in touch with an awul lot of code written by an awful lot of people (now don’t misunderstand this one, they were mostly nice people, but there was awfully lot of them), and one common denominator is that when not forced to, most of people don’t document, much the same way Mr. Pink doesn’t tip. The most common excuse is: “I don’t have time.”

Now I don’t buy it, much the same way Mr. Pink doesn’t tip. Just think of future time savings when doing an upgrade and when you won’t have to wonder what in the Earth did that developer think of. Oh, and if you are not going to do that upgrade, in this case it is about responsibility too: help others save their time, and you’ll be their hero. And finally, to properly document anything it takes anywhere between 5 and 7 seconds, which is something, I believe, we can all spare for a decent cause.

Thought of the day:
I’m a very responsible person. Whenever something goes wrong they always say I’m responsible.

Vjeko

Vjeko has been writing code for living since 1995, and he has shared his knowledge and experience in presentations, articles, blogs, and elsewhere since 2002. Hopelessly curious, passionate about technology, avid language learner no matter human or computer.

This Post Has 4 Comments

  1. Jason Rakowski

    I found your site on technorati and read a few of your other posts. Keep up the good work. I just added your RSS feed to my Google News Reader. Looking forward to reading more from you.

    Jason Rakowski

  2. infonote

    I agree 100% with your article.
    Pity with Navision you cannot use a versioning system like VSS.
    Even with a versioning system this should be done but it will help.

  3. Dave Roys

    Another great post. Good advice for all programmers of NAV.

    So what does the code do? Well I think this is a perfect example of code that is devoid of usefult comments of intent. If I had to stick a comment before this code it would go something like this:

    // Copy the first GLEntry for each GL Account to our Temporary GLEntry table.

    Do you have a big stack of posts just lining up ready to post or are you an insomniac?

  4. Vjeko

    Jason: Thanks a lot, I appreciate it, and I hope I make it worthwile for you.

    infonote: As the matter of fact, there is something that is in my queue to blog about, maybe I do it next, so stay tuned.

    Dave: Thanks again, pal! Regarding code, you are almost there, and I’m glad you didn’t just get there, because this proves my point – this code does a bad job at being self-explanatory. Actually, the intent of the programmer was to simulate SELECT DISTINCT, but you don’t just read it right away. And no, I don’t have a stack of posts, I am an insomniac 🙂

Leave a Reply