So I had heard about the book "Clean ABAP" before but I didn't know that there's an online version.
I do read a lot about coding practices and so it was interesting to read "Clean ABAP" as well - and I do agree with most of what they say.
What I don't agree with is the section on "Comments", though. And that felt like a glitch in the Matrix because I usually find that coding style guides make a lot of sense to me - even the rest of Clean ABAP does - just the Comments part doesn't. At all.
They explicitly say they're building on top of R. Martin's book "Clean Code", so it's for people who are familiar with this book. I had never heard of it, so I don't know if that is a standard book on the topic or just one of several opinions. And what that guy says is, in essence, that comments are at best a "necessary evil" and that you should "cringe" every time you write a comment because it means that you've failed to write "clean code". "Clean ABAP" phrases this as "Express yourself in code, not in comments".
There are even some hard-core fans of this who tell you to stop writing comments altogether.
Their reasoning goes something like this:
- Comments are "traps" because the code will be adjusted in the future but nobody will adjust the comments.
- Comments are an excuse for writing bad code (you can't be bothered to write understandable code, so you explain it using comments)
For proof, they usually cite 4-line code examples where they show that you can omit the comments just by better naming
I couldn't disagree more.
First of all, they make some inherent logical mistakes:
Re 1: So all these guys are promoting a clean programming style. Meaning: This is how you should write your code in an ideal world. Wouldn't you expect that developers update their comments in an ideal world? The whole point of a style guide is to change or unify people's behavior. And if you think people write bad comments, you need to tell them how to write good comments. It's the same as saying: People write bad code, let's just stop writing code. That's not really the solution, is it?
Re 2: That one made me laugh. I don't know a single bad programmer who writes comments. In my experience, there's a directly positive correlation between code quality and number of comments. Also, everyone (including the bad programmers) usually think they're good programmers (hello, Dunning and Kruger!). So to claim that somebody would write comments because they know they're a bad programmer is a bit unrealistic.
But I think it points to the root problem here.
Option 1) Maybe I misunderstand the premise of "Clean ABAP". If the premise is to write minimal amounts of code, it might make sense. But then again, in other sections, they especially claim that you should write more code for the sake of readability. So I don't think that's it.
Option 2) These people are a bit arrogant along the lines of "well, if you're a good programmer, you're going to understand my 'clean' code without any comments". This puts pressure on especially younger consultants, who think "well, I'm the problem", which is very exclusionist.
And that's what I totally disagree with.
No, writing code is not a competition in code comprehension. Especially as SAP consultants, we're paid to deliver results in a corporate environment. And it's often very complex requirements we have to implement. And that's why your code should contain comments that make it easy for any new consultant joining your team to understand the code right away, instead of spending 2 days.
It's like saying: Technical devices don't need a user manual. If you need a user manual, you've failed to create an "intuitive" device. Even the smart phone settings are full with explanations of the various buttons and settings and there's a link to the help section. I'm not sure if the "Clean Code" fans would argue that manuals and explanations are only for dumb users. If they wouldn't, then why on earth do you think the same doesn't apply to code? If they would, then you've just outed yourself as an exclusionist bunch of nerds.
So here's my little style guide with regards to comments:
Express yourself in code AND in comments
Make sure your code is clear and concise. But also make sure you include a comment on top of every method or coding block that explains what you're doing and - most importantly - why you're doing it.
Citing their example about leap-years your comment could be: A date calculation could result in an invalid date (e.g. 31/06). This method converts this invalid date to the last day of the month.
When I read their "clean" example, I could not immediately figure out what they were doing. When I read the comment in their "anti-pattern", I immediately understood what it was supposed to do (even though they used bad grammar which made it a bit difficult to read). But yeah, sure, I'm probably just not as clever as they are ...
Use comments and NOT methods to segment your code
In my experience you should do the exact opposite of what they recommend. As a general rule, I would create a method only if it is worth it. In their example from the previous section they create a method "reduce_day_by_one" to do a 1-line calculation. Unless this 1-line calculation is reused somewhere, it only makes your code more difficult to understand as you have to navigate to the method. (Of course, if the code is longer, it usually makes sense to put it in a method, if it otherwise clutters the calling program - for instance, from a case statement it makes more sense to call a method than to insert 20 lines of code.)
I'm really not sure where their dogmatism is coming from. They are really creating a method with the name "reduce_day_by_one" just to avoid to write a comment "reduce day by one" above the formula? And they are seriously telling me that the "bad programmer" who would forget to update that comment (because the new requirement is to reduce to the last day of the month rather than just by one day) would seriously bother to rename the method???
I usually separate blocks of coding by clear comments. For instance, when replicating employees from ERP, I've got a class that contains a method "process". In that method "process" I handle different scenarios (based on the selection screen). Those scenarios are separated by comments. Each comment explaining what the scenario is. Putting those scenarios in separate methods would make reading the code more complicated and I would need the comments anyway because, let's be honest, a "beautifully crafted" method name is limited to 30 characters and, sorry, splitting 10 lines of code into 10 different methods (I'm exaggerating) just so I can give every line a name and avoid comments doesn't make any sense to me.
Also, especially when dealing with structures in ERP, it usually helps to add a comment that explains the (often German) field names. Commenting that the field "ANRED" (Anrede in German) refers to title doesn't hurt anybody and helps so many others. (Thinking of it, why don't we just abolish the description fields of data elements, reports, methods etc. in SAP altogether - if it's all just about proper naming, who needs a description?)
Write comments to explain the WHY and the WHAT
For someone who's reading your code for the first time, it's usually most helpful to understand WHY you're doing what you're doing. But you should also mention what you're doing.
Taking our example regarding the leap years above. If you just wrote "Reduce to last day of the month" you know what the method is doing, which is a good first step, but adding the explanation "because the date calculation may produce an inaccurate date" is what's most helpful.
It's also a good idea if you add a comment for anything you failed to understand immediately yourself. E.g. if you have to add coding to work around a quirky standard behavior, someone else may think that it's redundant (or that you were a bad programmer). Adding a comment explains why you're doing what you're doing.
Design DOES go into the code AND the design document
In theory, I agree with the "Clean ABAP" folks that design should go into the design document. But, welcome to real life, design documents are usually either not available (because they weren't done or they were done 10 years ago and nobody knows where they are) OR not worth the paper they're written on.
So it's a good idea to put the general gist of what your development does into the header. If you're dealing with a report, the top of the report. If you're dealing with a class, I'd put it into the constructor. This should largely explain the WHY and a short summary of the implemented process. Don't copy and paste the design document. The design document details the plan, the comments in the class are the "user manual", so to speak.
Especially when you're copying code from somewhere else, you should explain why you copied it (instead of calling it) and where it's from.
Also, if you code interacts with code in another system (web service call etc.), it's a good idea to insert the development object that's called in the other system. That will enable others to quickly see how the different parts interact with each other, without having to start analyzing in the other system.
I have to add that it doesn't help that the "Clean ABAP" "anti-pattern" examples are written sarcastically bad. If you need to distort the anti-pattern to a point where it isn't even a pattern (nobody writes comments like this, seriously) - you're not exactly proving your point.
DO manual versioning
It is always a good idea to include the change number as a comment in your code. Especially in large, legacy code that behaves a bit like a house of cards, it will be easier to see who made what change. Yes, you could check the version history of the source code and that would also tell you who changed it and what the change number was (if it's included in the transport request description), but good luck with that if you've got 30 entries in your version history, because you're going to have to compare them one by one.
Just putting the change number, is not a good idea, though, because the system where that change was described may no longer be available. That's why (see above) it's also a good idea to give a summary of what you're changing and why.
I don't agree with the notion that these comments pollute the source code. Especially in ABAP, source code versioning is a pain. You have to compare the versions and it's text based. In other languages/environments this may be different, but in ABAP, it makes it very easy to see who made what change (the when is not so important, in my opinion). And if you don't need those comments, simply skip over them.
Add message text to message calls
I think it's a good idea to add a message text to all message calls. It makes it easy to see for anybody looking at the source code what message you're raising and helps understanding your logic. I disagree that messages will become outdated quickly. Yes, the wording may change but if we have a message like "partner doesn't exist", it's unlikely that it will change to "partner changed successfully", in which case the original code (containing the message text comment) will have to be adjusted as well, and fundamentally.
Update your comments
The point that outdated comments are misleading is valid. So you should make sure you update comments when you change the corresponding code.
Delete obsolete code
Obsolete code shouldn't be commented out. Be brave and delete it straight away. For the rare case that the code removal was an error, we do have the version history and can always get it back. Tip: In case you find that a class or coding is so valuable that it might be needed later on, create a local copy in your dev system. That way it doesn't clutter up production and can be accessed in the DEV system whenever needed. More often than not you will find you won't need it again :-).
Be tolerant and put yourself in other peoples shoes
If you think you're a great developer and can write obvious code that's well-named and doesn't need any comment - good on you! You've made it! Now, how about considering those who are not as smart as you? Think back to when you were learning ABAP or debugging someone else's code and were happy for any clue that told you what the heck they are trying to do. And then just assume that the next person reading your code will think the same. If it helps, just pretend they're just too stupid to understand your beautifully crafted code. But the point is: Everyone's different, everyone's brain works a bit differently. What makes sense to you may make no sense to someone else. That's why we've learned to speak and communicate in natural language, to understand each other better. And since you're the smart one here, able to write "clean code": Share your knowledge, add comments that explain what you're doing and why you're doing it.
And if you manage to write complex code that doesn't need a single comment, I'm the first to be impressed if I understand it by just looking at it. I'll be happy to show it to a random selection of 10 ABAP consultants. And if everyone agrees that it's obvious the way it is, we'll keep it like that.
So, do what you're good at, be tolerant of others who you think are inferior to you and help them get better!
Final Thoughts
It worries me that "Clean ABAP" takes such an exclusionist stance on comments.
Rather than pointing out how bad comments are, they should have focused on how to write good comments. I'm always happy if I find source code with comments. I very rarely find comments that are misleading our outdated. And if they are, it's usually very obvious and then I'm off no better than if the comments weren't there at all. Put another way: If you're smart enough to understand code without comments, then why would you be bothered by comments in the first place (whether they're right or wrong)?
In that regard, anybody suggesting to me that comments are lame and only for people who can't write good code, shows that they themselves are not good programmers. Because good programmers are not only good at writing code. They're also good at taking care of others. By adding comments that help them understand the what and why.
P.S. The rest of the "Clean ABAP" style guide is worth considering. It's not a gold standard or standard book by any means. They're totally absorbed in the world of "New ABAP". So a lot of their suggestions will indeed make your life easier (like using line_exists instead of read table with key transporting no fields). But a lot of other stuff is very questionable, like what they say about classes (avoid using self-references), methods (create lots of method that are small, only have one export parameter). This stuff is not wrong, but it usually applies to certain scenarios while other scenarios may require a different approach. When working with others on code, I'm a fan of "live and let live". We agree on certain important standards (e.g. try to avoid redundancy, avoid nested coding so it stays readable, add comments, document what you did) - but the rest is up to the developer.
Micromanaging people and being a language lawyer by pointing out little mistakes as in "you could have optimized this" (unless it's indeed a measurable performance issue), "you could have named this differently" or "on page 134 it says you shouldn't do this" is not my cup of tea and wastes a lot of time. That way, everyone can leave their personal touch in the system and it truly becomes a joint effort. So with its dogmatic approach, I would recommend that teams consider what's said in "Clean ABAP" and use what makes sense to them - but to not treat it as a single source of truth.