25

As per the title, should I be explaining syntax that I use in my code when I have a reasonable expectation that a developer looking at my code in the future will not be familiar with it? Short caveats or explanations could be added in-line while more complex constructions would be referred to by name and version number to allow a quick lookup in the documentation.

The two main reason they might not understand the syntax are because it's a new addition to the language or it's syntax that is rarely used by "average" developers. Assume that using this syntax is the recommended approach and not about me wanting to write fancy code. (No, I'm not XOR swapping in high-level languages.)

On the one hand, comments are there to facilitate maintenance and if I provide that explanation I could save a future developer a lot of effort and reading. On the other hand, I could argue that syntax documentation is readily available and that any decent developer could be expected to keep up to date on new developments or, failing that, shouldn't have any trouble figuring it out using the tools (IDE help) and documentaiton available to him.

For context, this question was prompted by recent updates to SAP's ABAP syntax which adds new grouping and filtering mechanics to loops and a number of other statements. Many legacy developers and client-side maintenance teams won't be familiar with this syntax for years (if ever).


For this question, assume that commenting your code is a good practice.


As csw comments, one option is to simply not use this new syntax until the maintainers have caught up, but as I hinted, some of those maintainers simply won't ever catch up. In my industry maintenance of the code is often handed over to small in-house developers once the project is delivered and they can be notorious for being stuck in "legacy mode". Writing functionally obsolete code as a crutch for out-of-touch programmers just doesn't seem like a good or defensible practice.

In my particular situation, the ABAP syntax updates offer some dramatic performance and readability improvements. They introduce a number of common modern operations to the ancient ABAP language like casting operators. The GROUP BY I linked is another good example that replaces complex, illegible and bug-prone code with a simple alternative. For a concrete syntax example, I can replace

READ TABLE itab WITH ... TRANSPORTING NO FIELDS.
IF sy-subrc = 0.
    ...
ENDIF.

with:

IF line_exists( itab[ ... ] ).
    ...
ENDIF.

For those interested, this PDF covers many of the new features. Slide 29 and 30 offer two good example of how drastic the difference can be.


Regarding the suggested duplicate, there are some useful answers there but my question covers a more specific case. You could probably summarise my question as "Should I ignore the document *why* rather than *how* paradigm when when doing so might greatly simplify maintenance for inexperienced developers?".

  • 2
    I personally try to avoid comments as much as possible. Sure, comments may help in this instance, but they still carry all the usual problems as well. My recommendation: Don't coddle the future developers on the project. They will need to learn how this stuff works. Coddling them just slows them down. – MetaFight Aug 26 '15 at 9:54
  • 2
    Dont' worry @cxw , I expected to get that reaction and I understand where you're coming from. I've updated my question to explain the benefits of using the new syntax as they're generally quite significant. – Lilienthal Aug 26 '15 at 10:41
  • 3
    Comments should not explain platform features like syntax etc which is already documented elsewhere. They should only explain what is particular about you own code, i case it is not obvious from the code itself. – JacquesB Aug 26 '15 at 12:09
  • 2
    And I'm saying that if you're relying on bread crumbs to keep your system afloat then you have much bigger issues. – MetaFight Aug 26 '15 at 12:52
  • 5
    @MetaFight: Mistakes occur in well-designed systems, too, and in an ideal world someone with deep knowledge of the code is available 24x7. Reality says that's not always the case, and maintainability means maintainability by those outside the inner circle. I don't advocate dense commenting, but the occasional bit of narrative helps identify and skip parts of the code that probably aren't involved. – Blrfl Aug 26 '15 at 13:08
20

In the context of ABAP you can rarely expect everyone to be familiar with all the newest features because:

  1. ABAP has such a long history and the ancient language constructs are still encountered all the time (I am currently modifying a report which uses a logical database and makes excessive use of tables with header lines).
  2. SAP themselves rarely uses the new features to ensure backward compatibility with systems not patched to newer releases yet, so you rarely encounter the new features "in the wild".
  3. The SAP education courses are very expensive, so companies avoid sending people to any which they don't desperately needed for their work. The delta courses which explain new features are rarely considered essential enough.

When your organization is not up-to-date in the latest development practices, you might consider to make it your mission to get them up-to-date so that explanations of unusual syntax aren't necessary. When this is not an option (you have no time, no resources, and not feeling that this is your responsibility), you should either avoid the new features to not confuse the other teammembers or comment them.

Remember the rules of good comments:

  • The newbie explains the syntax
  • The apprentice explains what is happening
  • The expert explain why it is happening

Bad:

" This uses the new line_exist operator in 740 which replaces READ TABLE
IF line_exists( itab[ carrid = 'AE' ] ).
    PERFORM foobar.
ENDIF.

When I know that operator this comment does not help me at all. Also, I might laugh at you for calling it "new" because this might be the year 2028 and that operator was declared obsolete years ago. When I don't know the operator, you just sent me on a 10 minute Googling spree instead of helping me understand your code.

Better:

" When any position has the carrier 'AE', we need to do the foobar operation
IF line_exists( itab[ carrid = 'AE' ] ).
    PERFORM foobar.
ENDIF.

This tells me what is happening so I don't even need to understand what this new language feature does. When I didn't knew it before, you just demonstrated me what it does, so I learned something from you. And even when I already know it, your comment might be more readable than your code, helping me to understand your program.

Even better:

" Anytime there is at least one flight by carrier AE on the
" flight list, the foobar operation must be performed to prevent 
" all the foobaz from getting confabulated.
IF line_exists( itab[ carrid = 'AE' ] ).
    PERFORM foobar.
ENDIF.

It doesn't just tell me what is happening but also what purpose it fulfills.

27

Contrary to other answers, I would say no. The code should be self-documented as much as possible. Such simple things should not need to be explained in the code.

I do not know SAP, but a simple google query pointed to the SAP reference page to the line_exists function.

If the maintainers and developers are not up to date, then the company needs to develop a strategy to keep them up to date.

9

I think it depends on the situation.

In this example, documentation like this belongs in a README file.* If you consistently use the new syntax, your code will be virtually unrecognizable to a programmer who only knows the older style. Your maintainers will thank you if you warn them up front that they don't actually know this language. I would:

  1. Start every source file with a bold block comment that this file uses new syntax:

    ***********************************************
    * ### This file uses the NEW ABAP SYNTAX  ### *
    * ### introduced in NetWeaver 7.4.        ### *
    * ### Please see README.html for details. ### *
    ***********************************************
    
  2. In the README, briefly explain the syntax changes and provide links (hence README.html) to tutorials and references.

This has two advantages:

  1. You can reuse the README across multiple projects. Don't repeat yourself!

  2. Your code won't be half comments. It appears the new syntax will be present throughout each file. Explaining syntax, even briefly, wherever you use it will be tedious for you to do and difficult for your maintainer to read.

* Caveat: All I know about ABAP I learned from the links in your post. Please don't consider this an expert opinion.

** By the way, the above sample block comment is hereby licensed CC0 1.0 :) . Feel free to copy it if it is convenient to do so!

  • All content on StackExchange is automatically licensed as Creative Commons Attribution Share Alike per the terms of use. See stackexchange.com/legal – user53019 Aug 26 '15 at 20:40
  • @GlenH7 - yes - that is why the CC0 was important! I wanted the OP to be able to copy the comment if it was convenient to do so. See also the blanket license on my profile. Please don't edit any statements granting licenses - those are in addition to the CC-BY-SA. Thanks! – cxw Aug 26 '15 at 21:04
7

In this case, someone unfamiliar with the updates could simply look up line_exists and move along.

For something less searchable, you might want to leave a comment which simply names the new features you are using so that up-to-date documentation is easier to find. If you use these features in more than a few places, put such a notice in a readme file instead of in comments.

I would not bother explaining the syntax if there is already ample documentation from another source, save your effort for documenting the things you develop instead.

  • I see that I should have been clearer in my original post, this is indeed what I'm doing, for example "This logic uses the new GROUP BY logic from ABAP release 7.40 SP08". – Lilienthal Aug 26 '15 at 14:20
  • 1
    If the code is a scripting language that could be run on older systems that do not have these new language features available, then a quick comment would not hurt. For compatibility issues, though, I would definitely put that in a README of some sort, then communicate, communicate, communicate. – Greg Burghardt Aug 26 '15 at 18:27
3

Note: I am not very familiar with ABAP, so I am answering as if someone asked about using lambdas in Java, in the understanding that the issue is similar (AFAIK it is). If I am wrong, please inform me.

This does not belong to comments (part of the code) as it is an specification issue of the project. In fact, it should not be a programmer who decides to use the new features, but the project manager or even the product owner who will decide which version/s of ABAP the project should be compatible with.

Somewhere in the project documentation should be written something like "Supports ABAP X". Now:

  • If X is 1990, you should not use any features that are not compatible with ABAP 1990. If you feel that those new features give so much added value that a change of X to 2015 is desirable, talk to the people in charge of doing that decission so they update the project specs (or not).

  • If X is 2015, any new programmer will know that they must have some knowledge of the new features just to be able to read the code, it is up to them to learn (or request formation for) such a version of the language, if they are not still familiar with it.

1

In one respect I think this is a bit of an X-Y problem.

For the sake of my example I'd like anyone reading this to consider C++'s recent C++11 overhaul. In particular I would like to focus on the constexpr operator. It's purpose is to allow certain functions to be run by the compiler (e.g. a factorial function) at compile time, thus preventing constant values having to be calculated at run-time.

Say for argument's sake that I am in a dev team full of C++ developers who haven't read about any of the C++11 features (let's say we're in 2012 to make this scenario a bit more realistic). I take our int factorial(int n) function and add constexpr. Now I have to document this. Do I write a comment explaining why I've added constexpr or does that sort of thing really belong on the project change log?

I would argue that comments are for explaining the current state of the code, not the changes it has gone through. You do not leave a comment for every bug you have removed, you make a note of it in a change log or an error log. Likewise the reasons for making factorial a constexpr should be expressed in the changelog.

The comment should still be changed however as I have changed the behaviour of the function. It no longer operates at run-time but instead runs at compile-time. Hence I change the comment from //Calculates the result of the factorial operation to //Calculates the result of the factorial operation at compile time.

"Aha, you have just explained the feature" I hear people think. No I have not. What I have done is make the comment describe the behaviour of the function. What I have not done is explain all the caveats involved in the use of constexpr (i.e. I have not explained in what situations applying constexpr to a function results in a compilation error).

Had I changed some syntax to use a more readable form, I would not be documenting such a change in a comment because I would not have changed the behaviour of the code itself. Instead I would be documenting the change in the change log.

Now in response to some specific points:

assume this syntax is the recommended approach

If it is the recommended approach it should be used. The only reasons to not use it would be if it breaks something or is not yet widely supported by the tools you use (such as some of C++11's features).

On the one hand, comments are there to facilitate maintenance and if I provide that explanation I could save a future developer a lot of effort and reading.

And deprive them of learning the full ins and outs of a language feature? You don't give someone a spanner and leave out the fact it's adjustable. If one of the other coders misses an important fact because they've read your summary instead of the full caveats of a syntax feature then it will effectively be your fault if they introduce a bug because of it. Sure it will be their fault for not reading the documentation, but why didn't they read it? Because they learnt about the feature through a comment.

once the project is delivered and they can be notorious for being stuck in "legacy mode"

And by forcing them to read the documentation you are forcing them out of this habit. This is a good thing™. For what it's worth I downloaded the pdf, the new syntax is much more in line with modern thinking. Again, this is a good thing™, you should be encouraging the adopting of new features when they improve readability, performance or maintainability of your code. Time spent reading documentation is never a waste if the reader learns something.

  • Excellent points and something I'll have to consider. Not sure why this gained a downvote, +1. – Lilienthal Aug 27 '15 at 9:17
  • "C++'s recent C++ 11" :D – phresnel Aug 27 '15 at 10:59
  • @phresnel Technically even now GCC and Visual Studio 2015 aren't fully compliant. GCC is missing "Minimal support for garbage collection and reachability-based leak detection" and Visual Studio is missing 'Expression SFINAE'. Until the most commonly used compilers are compliant, I'd say C++11 is an ongoing issue. Then when they are compliant we can start complaining about the C++14 features they are missing for the next 2 years. – Pharap Aug 29 '15 at 15:54
  • Truly, yet you wrote about C++11 (which if course is done), not compilers thereof :) – phresnel Aug 31 '15 at 9:21
  • @phresnel It's recent enough that there are enough people that haven't familiarised themselves with the features it introduced. – Pharap Sep 2 '15 at 0:19

Not the answer you're looking for? Browse other questions tagged or ask your own question.