Blog

Hint: this is a post for developers, and mostly junior developers, those who are still learning how to code properly. I know, I promised not to blog about stuff like this, but I simply couldn’t help this time.

A friend of mine has asked me for help.

“There is this C/AL function I had to rewrite, now I end up with 106 BEGINs, and only 105 ENDs. Do you have any idea how to find where this missing END belongs?”

No, J., I’m sorry, I don’t. I feel your pain, though. This reminds me once again of legendary “whenever you are writing code, write it as if someone else will have to maintain it and their last name is Gotti, and they know your home address” quotation I already quoted once. I am sorry, J., that you ended up maintaining this piece of, well, code (yes, it still qualifies as such).

“It’s a single function of an add-on, which grew organically over the past five years…”

No, it didn’t grow organically. It metastasized.

While a lot of code gets to grow over time, especially with add-on modules which go through several iterations of releases for many different customers, it is not the excuse to write bad code. There are a few lessons about coding we can all learn from J.’s unfortunate experience of maintaining other people’s code:

1. Don’t just code away: think instead, and think first. Think about what you need to accomplish now, and what you might need to accomplish tomorrow. The sooner you start coding, the later you finish.

2. Abstract whenever possible: Abstraction is when you take a coherent piece of code and blackbox it away in a separate function. Then you call this function from wherever you took the code in the first place. This makes your code more readable and much more reusable (not as in copy/paste, which is how a lot of code gets reused, unfortunately) – you can call this function from other places as well, reducing the overall code length and improving understanding.

Look at this:

Replace it with:

Generally, this is better. And you can do an awful lot of these with just about anything. This was an innocent example, where four lines of code were basically copied and pasted. The more lines of code you abstract, the more you benefit in the future.

When to abstract? It’s hard to tell, really. It depends. But no matter what, always abstract OnPush trigger logic. Anything that has even a remote chance of being called from two separate places is a good candidate. If you catch yourself copying a piece of code and pasting it elsewhere, stop right there, mane an abstracted function, then call it from both places. I’ve recently worked on a project where I found exactly the same piece of code handling some custom entry table in exactly 73 objects. The only difference was that some filters were different. You’d be surprised – this wasn’t written by a rookie in the first place. The guy was a seasoned developer, who just thought he didn’t have enough time to do the abstraction in the first place, and didn’t think this would get copied to so many different places. Don’t let this happen to you. And check point #1, it applies here as well.

3. Don’t IF THEN ELSE IF THEN ELSE BEGIN END ELSE IF THEN ELSE: When code grows, it often does so because a case which wasn’t supported initially now has to be supported. Don’t just add another IF to solve the problem. IFs, especially the nested ones, are hard to follow and hard to understand, and can be a hatchery of bugs if your logic needs exclusivity. This can lead to trouble:

IF i = 1 THEN
  DoSomething(i);
IF i = 2 THEN
  DoSomethingElse(i);
IF i = 3 THEN
  DoSomethingCompletelyDifferent(i);

If you start with i = 1, can you tell for sure that the other two cases won’t fire? No. So you do this:

IF i = 1 THEN
  DoSomething(i)
ELSE
  IF i = 2 THEN
    DoSomethingElse(i)
  ELSE
    IF i = 3 THEN
      DoSomethingCompletelyDifferent(i);

Ohmy, here we go… imagine adding some more logic in the future and you can see where this gets to. CASE does a much better job, so why not start writing your code immediately with CASE if you expect there might be a branching in the future?

This is so much easier to follow:

CASE i OF
  1:
    DoSomething(i);
  2:
    DoSomethingElse(i);
  3:
    DoSomethingCompletelyDifferent(i);
END;

Okay, this was too obvious. But there is a splendid trick with CASE:

CASE TRUE OF
  i = 1:
    DoSomething(i);
  i = 2:
    DoSomethingElse(i);
  i = 3:
    DoSomethingCompletelyDifferent(i);
END;

Now if a fourth case arises, which should fire only if other three haven’t, but has nothing to do with variable i, then you can simply add it here and retain an easy to follow code:

CASE TRUE OF
  i = 1:
    DoSomething(i);
  i = 2:
    DoSomethingElse(i);
  i = 3:
    DoSomethingCompletelyDifferent(i);
  j = 2:
    DoAnotherSomething(j);
END;

Much, much more readable (and maintainable) than:

IF i = 1 THEN
  DoSomething(i)
ELSE
  IF i = 2 THEN
    DoSomethingElse(i)
  ELSE
    IF i = 3 THEN
      DoSomethingCompletelyDifferent(i)
    ELSE
      IF j = 2 THEN
        DoAnotherSomething(j);

4. Don’t have 106 BEGINs and ENDs within a single function. It’s not to say that a code can’t branch 105 times, of course it can (albeit not too often). It’s simply to say that in case you have 106 blocks of code, you are better off applying any of the rules above, you’ll end up with less blocks of much more maintainable and repeatable code. I bet that any time that you have 106 blocks of code (or 20 blocks of code for that matter), there are many candidates for code reuse, abstraction, better structuring or a combination of these.

There are probably many more lessons to learn from things like this, and if you have similar experience, or similar C/AL (or general) coding advice, please share!