Are comments bad or not?

[AntonVomFriedabach]

Relevant sections of the style guide
https://github.com/SAP/styleguides/blob/main/clean-abap/CleanABAP.md#express-yourself-in-code-not-in-comments
https://github.com/SAP/styleguides/blob/main/clean-abap/CleanABAP.md#comments-are-no-excuse-for-bad-names
https://github.com/SAP/styleguides/blob/main/clean-abap/CleanABAP.md#use-methods-instead-of-comments-to-segment-your-code

Question
Hi guys,

Great work on the Clean ABAP book, which I mostly agree with. But your comments section is really weird. It's like, I get where you're coming from and I agree halfway through, but then I'm like: Why does it have to be so ideological/drastic? Why do you recommend the exact opposite rather than finding the middle ground? It's like you're waging a war on comments, which are - in my experience - usually not to be found at all or not in abundance. I'm posting this in the Clarification section because it might be I'm the problem and just not getting it.

(Disclaimer: I haven't read your full book nor Martin's "Clean Code" (too expensive, unfortunately), but I appreciate the free online version and am happy to help make it better, if you like.)

Express yourself in code not in comments

The usual gist in other guides (including the DSAG recommendations you reference) is: Write good code, but also write good comments because the next person reading your code is not inside your head, so it will help them if you explain what you're doing and most importantly WHY you're doing it (what was the requirement?)

I did find Martin's comments regarding comments online, which boil down to "you should cringe whenever you write a comment". I think I understand where he's coming from - after all you should try to write readable code that doesn't "need" comments. And it sounds like you share that opinion. Only: I fail to understand why writing comments is a failure? First you go on about the "happy path" and all of a sudden such negativity. Wouldn't we want to encourage people to write GOOD code and GOOD comments? And shouldn't any GOOD code come with an explanation of WHY it's doing what it's doing? Even iPhones have explanations below every setting - does it mean they've just not found the setting name everyone will understand? You usually need a bit of context, in my opinion.

I also have to say that your anti-pattern here is not convincing. It's an example of particularly bad comment-writing mixed with bad coding. Still, I found it easier to understand what the 8 lines your anti-pattern are doing than figuring out your 3 methods. (I did actually have to look up what xsdbool means, never seen it before (thanks for teaching me something new :-)), and your zero_if_invalid variable was utterly confusing - zero_if_date_is_invalid may have been more precise. So rather than doing "anti-pattern" while cv_date eq 0 - you're writing a 3-line, confusing method? How is that an improvement?)

I challenge you that your "good example" is actually an anti-pattern and that a better version is (I haven't actually run this, just off the top of my head):

METHOD get_last_day_of_month.
" our date calculation may result in invalid dates (e.g. 31 June)
" an invalid date equals 0 in ABAP
" so reduce last 2 digits (DD) of date YYYYMMDD by 1 in a loop until we reach a valid date
" which will be the last day of the month
WHILE cv_date eq 0.
      cv_date+6(2) = cv_date+6(2) - 1.
ENDWHILE.
ENDMETHOD.

Why would I need to call 3 different methods to produce such a simple outcome? People have to navigate back and forth and if I use the same approach on all my coding, my class will quickly contain 100+ methods. Don't you agree that lots of methods create a complexity in itself that even the best naming will not help make sense of? So I strongly argue for some middle-ground. Yes, you could leave out the comments in my example above, it should be obvious - but why??? The next person looking at this method will immediately understand WHY I'm doing what I'm doing. And it helps them or saves them 15 seconds if I simply mention that cv_date+6(2) refers to the DD - no harm done.

I've seen people online claim that "comments lie because they become outdated". In that case, I would challenge you that your method names also lie. Because if someone can't be bothered to adjust the comment when they're adjusting the date calculation - why would they be bothered to adjust your "method names"? And the bonus with the comment is, that it's longer, so you'll see from the context that it's not doing what it says it is - with the method name, you don't have any context and no clue if the name is wrong or the implementation. Half the ABAP developers I've been working with RARELY comment their code NOR use object oriented programming. So I'd argue that the people you're going to reach with the "use proper method names" paradigm - you'll also reach with a recommendation to "update your comments". And the people you're not reaching usually don't write comments in the first place - so this section currently tries to solve a problem that doesn't exist (people writing bad code and lots of comments), while not solving a problem that does exist (people writing good code and no comments because they think everyone's as smart as they are). :-)

There's lots of other reasons where comments make perfect sense and are not a "failure". ERP table names contain lots of 5 character variable names. So if I'm reading the customer title (ANRED), what's the harm in adding a comment "title"? Why should I implement a separate method get_customer_title just to avoid writing one little comment? And especially in ABAP, where - unlike other programming languages - we're limited to 30 characters in our method names and which doesn't support CamelCase.

Comments are no excuse for bad names

I totally agree with you on this one, but I'm afraid it's missing the point. I've seen my share of bad code in the past. (Bad code is code where I have to spend hours to figure out not only WHAT they're doing but WHY they're doing it.) Bad code is usually NOT commented at all. It's just a maze of spaghetti code. I've yet to come across code that's totally bad yet very well commented.

So, in all likelihood, the person writing your anti-pattern about "check_table" wouldn't even bother putting a comment on top :-).

Use methods instead of comments to segment your code

Again, I totally agree with you - half way through. I embrace and cherish the beauty of object-oriented programming. Putting things in methods, reusing them instead of copy and paste. I loathe 3000-line user exits - and I'm usually happy if they contain any comments.

But the problem here is usually not that people have "chosen comments over methods" - they usually have no clue about object-oriented programming, so they've done what the always do and more likely than not, they won't even bother with comments.

So again, this section is missing the point. It should read: Use methods to segment your code and comment them well. A recent good example for this is mapping employee replication from ERP to C4C. I've got one method in my class "do_mapping", one of about 27 methods. It makes perfect sense to me to map all 50 something fields inside this one method. And it makes perfect sense to split up the mapping of different areas using comments (header fields, address details, identity details) and explaining some of the fields (e.g. some of the fields are only respected by the receiving system when the employee is created and ignored on further replications - how do I put this into a method name without comments?). Why would I call a separate method for each of these sections? The method is doing one thing - mapping. Some of these mappings are using a bit more complex logic - those are of course in separate methods (like get_timezone based on user's country).

When I'm coding AND commenting, I'm always thinking about the next person. I'm not thinking: look how clever I am, I can write super fancy ABAP with no comments. I often have to stop myself from using too much of the "new" ABAP (nested inline functions) to keep my code understandable - and in those cases I naturally add comments because if it took me some time to figure out how to code this - imagine how much time it will take the next person (or me in 2 years) to understand what the heck I've been doing.

So when I think about the next person in my "mapping" example above, I'm imagining they're investigating a problem with the mapping. They would hopefully look in "do_mapping" and there use CTRL+F to find the field that's causing issues. Or search for something like "address" if the address line is causing issues - and hit one of my comments. Or they would find it helpful that my comment explains that C4C currently only supports "Mr." and "Mrs." as form of address - which is why we're only mapping those 2. (If you say: There you go! If that ever changes in C4C, your comment will be outdated. - I would answer: So what? I'd rather have C4C fixed and an outdated comment in ERP - than C4C not fixed and an external consultant banging their head at why the heck title "Dr." doesn't replicate. You can't have it both ways - and your style guide recommendation should be "adjust your comments when you adjust your code".)

So, in summary, I think the "maintstream" recommendations regarding comments have their merits and that it's not a good idea to try and reinvent the wheel around this topic in Clean ABAP. I'd appreciate if Clean ABAP contained a section on "how to write good comments" - rather than the "comments" section declaring comments as failures and explaining how bad they are (using bad examples). I've still learnt something for myself from your Comments section and will keep a critical eye on the comments I write in the future. As you would have guessed by now, I do like writing, so let me know if you'd like some help on this ;-).