Disclaimer: this is a rant.
Firstly, let’s agree that comments in source code are important for maintainability and readability of the code. I’ve heard opinions that the comments are mostly useless because they get quickly out of sync with actual code and these out of sync comments cause confusions. Who updates comments when doing some bug fixing? Let say that we agree that there is a value in comments and one of our goals while programming is to write a readable and maintainable code.
The reason why I am writing this rant is that many developers do not know how to write a good comments and they cause damage with their useless comments. By damage here I mean lowering readability of code. At least to me it’s important to see as much code as possible and useless comments are taking precious space.
Let’s start slowly and answer an easy question. Who is target audience for the comments in source code? End users? Nope, they don’t care. Project managers? Sadly, most of project managers in SAP world do not care about quality of code (let’s not digress and talk about useless PMs :-)) but even good project managers are not going to read your comments. Functional consultants? Maybe, but most of them don’t read source code. Developers? Yes, that’s it (I said that it’s an easy question). Hence an author of comment needs to optimize for the developers. By optimizing for developers I mean that the comments should provide some value and they should definitely not cause any damage.
Examples of Useless Comments
* Increase quantity by one
ADD 1 TO mseg-menge.
I know, this one is ridiculous but sadly I’ve seen this in real life. The purpose of comment should not be to document what code is doing. The code is pretty clear and exact in what it is doing. It should document why it was written. Remember, the target audience is the developers. They know how to read code and what ABAP command ADD means. Hence the comment above is completely useless and should not be there.
* Refresh screen
* Load data from DB
In this example, the comments do not provide any additional info. The names of the methods should be descriptive. When I see a method with name REFRESH_SCREEN than I assume that it refreshes screen and I don’t need to read it from comment. Hence above comments are useless. If names of your methods/routines/function modules are not descriptive than you have more serious problems and comments are not going to help.
* Create helper class
CREATE OBJECT wd_this->helper_class.
Here again, the comment above does not provide any additional info. I expect that variable with name helper_class holds a reference to an instance of helper class. Every developer knows (should know) what ABAP statement CREATE OBJECT does. If they don’t not know than they should not be changing the code. The comment does not explain what helper class is or what it does. Hence the comment above is useless.
And now let’s be more controversial.
* Begin of Change <AUTHOR> <DATE> <TRANSPORT>
* Remove old buggy code
* CLEAR mseg-menge.
mseg-menge = 1.
* End of Change
I really can’t understand why somebody needs to write all these additional lines that significantly decrease the readability of code. Sadly, it’s often mandated by development standards. Has anyone checked some big open source projects such as Linux kernel or FreeBSD? Can you see there comments like this one?
/* Begin of Change: dteske – Sat Sep 29 03:41:21 2012 revision 241042
Add missing IFS
f_clean_env –except RC_CONFS RC_DEFAULTS
f_clean_env –except IFS RC_CONFS RC_DEFAULTS
/* End of Change: dteske */
No, you definitely can’t see lines like this. The source code is not supposed to hold full history of changes and meta information about changes. All the information is stored in version control system. But wait a minute, ABAP AS has a version management where you can see who made the change and when it was made. You can also compare different versions to see what was changed. The SAP version management is far behind of new version control systems like Git but it’s still better than polluting source code with senseless comments.
I know that I haven’t shown you examples of good comments but rants are not usually helpful. I also believe that just eliminating useless comments will make the world better. So don’t forget. Every time you write a useless comment a kitten dies.
Good Article Martin... 🙂
"Every time you write a useless comment a kitten dies." - Can't stop laughing, just made my day.
Unfortunately i had to work on an HR user-exit (>5000 lines of code) once, of which more than 50% code was commented out 😡
A change history section at the top of the code (we say flower box 🙂 ) should suffice. A sane ABAPer will always use the version mgmt. to get the details of the change!
recently I've seen a web dynpro method with over 900 lines doing lots stuff and I couldn't believe it. Why wouldn't somebody split into into smaller code units such as function modules or methods. Anyway, with over 5k lines you are definitely winner. Commenting out whole section of code is not that bad because you can just skip it and read current code. My biggest problem is useless comments mixed with active code. Usually really hard to read.
Especially because adding or removing a method call is much neater way of changing / fixing / enhancing the source code. Unfortunately there are too many average or "lost" developers out there... but we can't do anything about it... *sigh*
A sad recent experience. I just saw a method with over 500 lines and raised it as an issue with colleague. He asked what it does. I told that it has over 500 lines of code so it's not really obvious from skimming the code.
The colleague who wrote it asked what it does?
No, he isn't author of the code. But I would not be surprised if author was confused by code after few months.
I can relate to that... can you believe the Project I'm currently working some folks created a class with over 3000 lines of unreadable code which has at least 4 layers bleeding all around?
pro tip: ends with customer.
Yeah. Just recently I've seen Java patterns ported straight to ABAP. Not so good idea either. It's quite hard to cut down number of lines in ABAP. It's quite expressive language. I think main goal should be trying to minimize size of the methods.
I hope my comment has same level of hidden meaning as your.
I think you got confused there... patterns are not directly connected to a programming language: instead, they are proven ways of dealing with problems of a certain nature... for instance, Data Access Objects, Business Objects, Data Transfer Objects, Service Facade.. those are the layers where patterns are usually applied.
Also, there are a lot similarities between C# and Java nowadays... in fact, if you follow 'Horst Keller' talking abap kernel and all new additions.. well, now we can use ADT (eclipse), many new additions on abap language are following languages like Java and C#, we are getting a lot of new things in there to code ' just like Java ' -- so I reckon it's a natural evolution, considering the representative numbers of each language... not to say, there are other things like EL, inline and so on.. very much like the language you just mentioned.
About the size of classes and methods: if we apply the single responsibility principle (creating a class and using plain procedural code inside it does not make you OO) - one will most often end up with smaller classes and more manageable source code... for instance, I would be surprised if you've seen these so called "Java Patterns" having classes with 3000 lines, or huge inexpressive methods like you get to see with normal procedural abap code.
And all in all, we're just two people talking of stuff we see on a day-to-day basis, I think it's a rather nice talk and I always get to laugh or smile on some responses I read..
What I meant was adding some methods that you end up never using (even comments in them say that they are not implemented). There is also a big difference starting from scratch and integrating with SAP code base that is based on procedures.
It so happens, in some projects, the coding convention followed is never to delete/remove existing code but to comment it.
I believe the better approach would be is to do a version comparison and understand the code changes.
The developer has to follow the rules – ending up in 50% of comments and 50% code. I have always liked a neat piece of code, with only code specific inline comments.
Like what you said, in a couple of months the actual author of the code, would be surprised to see the whole bunch of changes in the code…
Good rant, Martin. However, I also have a huge issue with excessive blocks of commented code. The older a system gets, the bigger the problem usually gets. I often come accross literally pages of commented code, with the odd line of active code getting lost in the sheer amount of noise around it. Often I'm not even aware a line of code is there until I try to step through it in debug mode.
Comments should give you context into "why" and "how" you're doing something, the "what" should be self-documenting by using good naming conventions etc.
That's exactly right - "why" and "how". Just the other day I had to deal with an old report that had some strange conditions in it. Generations of developers added comments explaining in plain English the obvious IF statements, but not a single world on what was their purpose. Gee, thanks a bunch. 😡
If the complicated "if" condition was part of a method, then DecomposeConditional could be applied.
Then, the condition gets a home for the future, and a good name which is better than a comment. The code which uses the condition reads more fluently. Detailed comments concerning the if statement itself can be banned into the method implementation, without disturbing the flow of the next higher program level.
OLD (Just fantasy, not real 🙂
if gs_tvak-klimp ca 'ABC' and " credit limit relevant
( gs_vbuk-gbstk eq 'A' or gs_vbuk-abstk ne 'C' ) and " doc open, but not rejected
( gs_vbak-netwr > gv_limit ).
if credit_limit_exceeded( ) eq abap_true.
ev_exceeded = abap_true.
I know the maximum method name length of 30 characters sometimes is a challenge for finding a good name. This is a limitation of the language. But still I think this pattern can be used with advantage.
Someone wanted to shoot himself into a leg or pay the same favour the next person who comes to work on the code. Or the person knew he would get hit by a bus and this was a way how to pay the world back. 5000 lines? OMG. A sane person would make that a set of classes I guess 🙂
customers are least interested in code comments... they know its the abapers job and it will be done... abaper need to put comments to help future abapers.
I mean.. what kind of abaper wont understand abap?
* clearing everything <<<< which abaper wont understand what a clear statement does
CLEAR: lwa_knaa, lwa_knbb.
*Fetching the customer information << now this makes a bit of sense sense
FROM KNA1..... etc
* Calculating the delivery date << this seems fine as well
lwa_bldat = lwa_orddt + lv_ddays.
That's exactly the point the OP wanted to make.
Recently i made some changes in an update module where i deliberately did not handle an exception. I wanted the update module to fail & the error to be recorded. If did not mention my thoughts in the code then a few months even i will forget why i did this.
The idea is to document "why" the statement was written rather than "what" the statement does. IMHO you cannot coach this to someone, this is an inherent quality.
For e.g., "why" did i use a SORTED TABLE with secondary key, "why" did i define an interface param with "pass-by-value". I will appreciate if a developer documents this rather than writing a sh** load of "useless" redundant comments.
➕ For e.g., "why" did i use a SORTED TABLE with secondary key, "why" did i define an interface param with "pass-by-value". I will appreciate if a developer documents this rather than writing a sh** load of "useless" redundant comments.
❗ but such details might make the programs bulky
Would you care to define "bulky" for me?
I have seen code where developers have written quite a few lines on why a COMMIT was issued at a specific position.
➖ Would you care to define "bulky" for me?
bulky simply means bulk of hours consumed. I am not talking about disk space here hope you got it already.
ℹ But first can you clarify if the customer is really interested in the junk we write in abap coding?
ℹ Will the customer ever read the documentation that was made to support future developments?
ℹ Can you give me a rough percentage of how many developments you have done where you have created an extensive document explaining the technical considerations?
ℹ Was the documentation client billable?
ℹ How much percentage of delivery hours do you spent on an average to comment/document your codes.
Commenting has wasted client billable hours which serve as no reference to future abapers. The first thing an abaper does it switch to debug mode and fix it. For breakfixes and minor support generally 2 - 4 hours are allowed and if that time is spent on reading documents then god help us.
As far as I understand there is no need for any kind of documentation in the code at all. Are not we already creating a Technical Specification? Of course we do. How many abapers who work on support tickets really thoroughly read the TS (leave along in-line comments and documentation).
I have seen SAP developed abap reports giving the worst nightmare in extended program checks filled with all junk logic, so I am really not sure about the expertise of people with ages of ABAP experience. They developed a SLIN that they dont follow themselves. 😕
I logged in after a long time and found your comment and point you are arguing about. A simple question you have got a report developed by you only 1 year ago. It contains like 10000 lines of code. Today you are reported to fix a severe performance bug or simple one. Now tell me how long will you take to do it..Assuming you dont want comments as quoted by you in earlier comment.
"As far as I understand there is no need for any kind of documentation in the code at all"
I believe after one year you wont even remember why you created this report in this way and you will never be able to find since you have not maintained meaning full comments not the Useless one which the O/P wanted to convey through this blog.
There is no need to remember because
1. I would develop the report when I am working in implementation project when creating it
2. It is the responsibility of the support team to figure out what the code not me.
3. If I get to work in a support team for the same project work and that same report is somehow assigned to me for breakfix I would first download the TS and find tha appropriate section where the breakfix has to be done will straightaway debug the respective area and find the solution.
Maintaining IN_LINE comments by developers is defined in the development guidelines set by the customer for which we develop / change the abap codes for. If it is mentioned in the guidelines then you can be sure that this is also a part of delivery (not just the abap code). Some customers hate IN-LINE comments. They want to keep things clean. Some customers have custom developed code inspectors where it asks the developers to add/remove comments as well.
I have already shared my points:
* clearing everything <<<< which abaper wont understand what a clear statement does
CLEAR: lwa_knaa, lwa_knbb.
*Fetching the customer information << now this makes a bit of sense sense
FROM KNA1..... etc
* Calculating the delivery date << this seems fine as well
lwa_bldat = lwa_orddt + lv_ddays.
So request you to read all my comments before reply.
How will you find breakfix needs to be done from the TS..?You will remember this is the place which must be causing the issue.? If you will remember then why did you code in this way? 😆
"I would first download the TS and find tha appropriate section where the breakfix has to be done will straightaway debug the respective area and find the solution."
Gaurab please have some facts before putting them here..read on SCN..learn on SCN..so many senior Guru(Baba) of ABAP you will find who still wants to learn ...Share on SCN..help people on SCN....rather than making such USELESS COMMENTS here.
Your comments are as good as SPAM
Very poetic. But does not add any to value to this SCN does it?
I suggest and do a extended program check(SLIN) in standard SAP code and you will understand why Gurus still need to learn.
Funny. You think that SAP must have been written by programmers with years of experience. Since the code is often shockingly bad, you assume that means that having years of experience means little.
Your theory though doesn't take into account a few things.
1) Much of SAP has been written by newbies fresh out of college
2) Much of the older reaches of SAP have been written before good coding theory was developed (and before SLIN or Code Inspector) - it's been around a long time.
3) SAP is in the business of rolling out software to customers. Some sections within SAP fail to understand that shipping bad code ultimately cuts their profits, preferring the short-term approach that many companies continue to apply to their IT strategy.
That is what I have been trying to put forward for some time.
1. There needs to be proper instructions in the development guidelines otherwise the freshers would not be able to deliver good programs with proper commenting wherever required.
2. Customers often choose to make Z programs because the SAP solutions have not followed good coding standards which cause performance issues as well as dumps due to data volume, or sometimes altogether do not meet the customer requirement for the same reason.
3. SAP Partners (IT companies) are the ones who generally follow the good coding standards while sap tends to ignore it. But ultimately SAP gets the loyalty from the IT companies. So its a win win scenario. IF SAP starts delivering good code then the SAP partners will be out of job. This would effect the loyalty program.
Your glass if full bro...look at how Guru's are still having empty glass and always ready to help and learn..
Time to stop replying to you and move on to something productive
You're clearly rather inexperienced - or experienced only a narrow way of working.
1) Any developer with the attitude that "I don't have to write good code, because it's the support team who have to figure things out" would very rapidly find they were no longer working on my support tem.
2) While the environment you work in may have proper documentation standards and procedures, it is certainly not uncommon for documents not to be available or never to have been written.
I've worked for many companies over the last 25 years and have never encountered one that "didn't like inline comments". It is a ridiculous attitude to hold. The value of commenting is well established in the IT industry.
A good yardstick is to imagine that the guy who is dealing with your code after you is a violent psychopath... and knows where you live. Code and comment accordingly. Any programmer with integrity will develop code that can be easily maintained by their successor. Anything else is cavalier and irresponsible.
1. would have appreciated if you had taken up the rest of the points rather than making up something from your own mind.
1. Please tell me where I had said I don't have to write good code, because it's the support team who have to figure things out" . If it is entirely your creation then please take ownership of this rather than putting it on me. As for me all my deliveries go with 0 error 0 warning on SLIN with minimal use of pragmas or pseudo comments. I use in-line comments only to explain functional necessicity and not for explaining the tenchical jargon which will obviously be understood by abapers.
2. Programming standard documents are developed by ABAPers to set some rules and guidelines on how to write comments as well. If that section is missing you need to work at it to add the section in the development guidelines. If the entire document is missing then I would suggest creating the same as a part of Application Renovation as it is good for the health of the project.
As I had already told I would not stretch again on it. Please go through all my comments on the topic before commenting or making assumptions and gang up on me.
The people who made SAP also made some useful comments like the ones below. Probably they are having the same kind of industrial experience now 😆
Amusing Comments | SCN
Funny comments in Standard SAP programs!!
That's what I already had suspected: you didn't get the point of the blog post. It's precisely the useless comments that are under fire there.
"A sane ABAPer "
Sane ABAPers were discontinued in ECC5.0, please update your documentation.
When referring to ABAP, please use the Basis release number. 🙂
With regards to the version history - I have seen cases where it is removed from complex code - I think there is a basis option where version history can be wiped - and I have also seen this happen in the past after upgrades etc. I think once you have seen the version history getting blown away on several occasions to a really complex user exit / enhancement you start to see the point of the commenting version history. This is one area that SAP don't really seem to have taken huge leaps and strides to improve - unless things have changed recently that is. I would love some really good code version management tool - something like what basis technologies are offering (but for free with the SAP system)
With regards to kittens - these don't really appeal to me so greatly either - so I guess more useless comments for me then 😉
you are right that SAP seriously neglected development tools. Fortunately, we got Eclipse recently. At least we get some basic stuff such as refactoring. Eclipse is not going to help with version management 🙁
I thought that everyone likes kittens.
I believe that many cats have died because of me ... that's not true, it's a joke.
One of my clients has the policy of commenting out dead code, instead of giving it a decent burial. So far, I've avoided it.
I'm fairly sure the additional cost of commented out code more than outweighs the low risk of complicated old code versions being lost and it being important. Complicated code can of itself be an indication that something is wrong and a refactor is needed.
I did get told off once for not using enough comments. I just politely asked what comments they would suggest for:
I can feel your pain. Commenting code just for sake of commenting is useless. Well structured code usually does not need too many comments. You want to comment just some weird quirks and workarounds.
Next time to avoid useless comments you can say that you like kittens 🙂
And it will be the same clients who insist on reams of technical specifications which do nothing aside from list the custom objects referenced by a piece of development.
Don't get me wrong, I'm a firm believer that a good technical spec has it's a place, but blind insistence that a document must exist, regardless of whether it will ever be of any use to anyone is a particular pet hate of mine (right up there with comments which make me wonder if the developer was paid per asterisk. And kittens).
There is a difference between a specification (why, who asked for this, what is the reason the standard is not enough, where is the code copied from etc.) and a list of objects (that is anyway only created once the thing exists in the system to some extent).
Because the developers ignore the rules and don't list the objects they know may not survive the first encounter with the "enemy", it is a copy of the history. Especially on the big projects where these lists of objects are required -> total nightmare. Even worse when you have analysts creating these things that are not responsible for the real implementation -> they often don't consider too many things including the sanity or the level of the actual developer 🙂
Pity there are so many policy rules that can't be enforced (or not easily or cheaply at least) and others that kill kittens that are followed so well 🙂
Excellent blog - describing our daily pain when working with "comment-polluted" code!
When programming, there are two actions when I take a step back:
ad 1) Why Copy/Paste? ABAP provides many ways to avoid code duplication, from Macros up to method calls. I consider extracting the source of my intended copy/paste into a reusable code module. The effort to parametrize the code usually pays off very well.
ad 2) Why comment? Couldn't I write intentional code, i.e. code that expresses precisely what I intend to do? And if so, why was this? What is the root cause for my desire to write a comment?
For example, a comment like
* Now start processing the input IDoc
indicates that a group of the subsequent lines can be summarized under a certain function. But then, these lines of code would better dwell in an own method, properly named, and to be called at this place, like this:
It's all about this bad form of laziness [there are good forms of laziness, too!]: the unwillingness to create methods which, by their name, indicate precisely what the code does.
In some other cases, a comment indicates a limit of expressiveness of the language itself: When the statement to use does not by itself explain the intended action. But also in this case, such a statement should be hidden in a well-named method. It's an implementation detail.
What really changed my way of thinking in these regards was Robert C. Martin's book "Clean Code".
I dont know why, but many of the standard coding guide lines client follow is to comment the whole code which needs to change and then write your code below or above it , after 3-4 issues the whole program contains 3/4th of the dead code (excluding the declaration) and half of the time programmers just comment some part of code in between lines and add their codes in between like doing a patch up ... and when this program is read by another abaper for an issue he does not have a clue what this code was supposed to do and then , with the help of debugging he fixes it somehow at some specific parts of code .... Worst happens when a code like this comes to you for optimisation issue 🙂 ...
Personally, I find the <begin change> / <end change> comments to be the most useless and annoying. But if you feel that development standards are not right, why not change them? In my current job there was so much nonsense when I just joined, but I proposed a different solution, the whole team voted and nonsense went away.
As a consultant, you might not be in a position to argue with a client, but still I'm sure any manager would be open to a suggestion that improves productivity and reduces total cost to own (and removing pages of old commented code does just that).
What solution you have proposed kindly share here to develope with all 🙂
I collapse comments when they are useless.
Context menu > Outlining has that option but it is not have any default key binding.
I have mapped Ctrl+Num0 to it.
When a procedure is big, it is better to collapse all and then selectively expand.
Unfortunately the problem mostly comes from the abapers themselves, since most (I will dare) lack initiative and critical thinking.
Its strange that in the SAP world they are taught to leave the thinking to the functional consultants. This leads to the survival of many low quality programmers because the standards are too low (they don't have to think.....).
This lack of initiative leads to the survival of stupid behavior like the ones described in this thread. No one questions the usefullness of making X.
Just a rant 🙂
that's an excellent point. Another thing that is bugging me is using these prefixes like ls_, lv_ and so on. Pick up any random project on github and you won't see it there. What really gets me is that when I find variable ls_po and it's global. So a developer has no idea what ls_ stands for and he just follows some old rule without any understanding.
BTW if you want an example of opposite extreme then watch this great presentation.
SAP shares some of the blame on that one, since old SAP seems to have been programmed in the dark ages. Global variables, little encapsulation, it's horrible. You want to use some functionality of standard SAP screen but you can't because the business logic is attached to the UI.....
Worse, SE51 forces programmers to use global variables..... I avoid SE51 because of that, but many, many....well most, don't know webdynpro so they keep using it.
Nice post Martin,
enjoyed reading the blog!
And suggest you to read Stackoverflow's compilation 🙂
What is the best comment in source code you have ever encountered? - Stack Overflow
Good rant Martin Voros as it helped few kittens share their plight and thank you for pointing out at the mess that we usually create while developing but don't care.
Empathy is one of the most important quality of a developer.
I do not agree with your last point about comments about changes.
It will definitely be a help to see where code has changed. Although it might be better to keep it at the beginning of the source code.
If a problem arises at least when you are debugging or analysing the code you know there has been a change and that wil trigger you to check the versions for that particulary piece of code. It has been helpful to me in programs of thousands of lines in complex SAP environments.
SAP standard code is not Always as descriptive as when you write it yourself and then this can really make a difference.
I do agree that some comments are useless, but who cares ? I have no problem with that. I have seen many useless comments in standard SAP code as well.
But that is my humble opinion.
in that case I envy you. When I am looking at code, especially not written by me, I really want to see code and nothing else. It's not that bad if big chunk is commented out because I can simply skip it. But when there is old code interleaved with new code it gets really messy and structure of code can be lost. Somebody mentioned that it's possible to bind automatic collapse of comments to keyboard shortcut. I think it's a great idea.
Regarding changes in code. I agree that sometimes it's really useful. I like when SAP developers mark lines that were added by specific OSS note. I find this really helpful because you can just check OSS note for more explanation. The comments are also at the end of the line so they are not really distracting.
I also do not agree that we should just ignore useless comments. I think it tells a lot about how much developers care about code. I follow FreeBSD project. If you follow SVN logs of this project you will very often see commits that fix spelling mistakes in comments or commits that fix non-standard formatting. I think this really shows how the project tries to achieve high quality of code.
BTW I also believe that pretty printer should be mandatory. Similar what Python or Go do. I presonally always run pretty printer before activating code.
It is good that pretty printer runs on custom programs even in display mode and collapse comments works while debugging. I like keyword uppercase, but some programs either have keyword lowercase or no pretty print. In those cases, it helps to pretty print and collapse comments using 2 keyboard shortcuts.
I like keywords upper case as well but I would not mind other combination if it was mandatory.
I understand your point. If there was a recent change in code, it is helpful to know about it without having to look it up in the version database (e.g. in a second session).
But on the other side, after some weeks or so, the tagging comment "* Hugo changed this" is nothing more than pollution of code. The comment remains in the code forever, although, once the correction proves as really stable, it is of no need any more.
Just dreaming: A solution could be in the IDE: When inspecting code or debugging it, the IDE highlights or marks code lines in a special manner that have recently been changed. Where the precise definition of "recent change" could be customizable in the workbench settings, with a reasonable default value.
That would be a nice solution!
that's a great idea and I think easily doable in Eclipse. Just a keyboard shortcut that retrieves previous version and highlights lines that were added. I think one of the main reasons for leaving code there is that SAP version management is not so great anymore. These enhancements which can be implemented in Eclipse might help to overcome these issues.
Besides the previously discussed issue where there is a possibility that version management may get lost, what else are your problems with SAP version managment? It certainly isn't as powerful as a distributed version control system like git, but it doesn't seem to be that badly broken either and I'm not sure if distributed version control is suitable for ABAP.
first, I want to say that SAP version management was really great when it was developed. I really want to give credit to developers who built it at that time. There are few things in ABAP that developers got really right.
What I really don't like is when you have to resolve conflicts. For example one developer is working on an object and her changes got into QA for testing. Now there is a need to push quick fix to production. You have to back up new changes, push fix to QA and production and then put back changes. I am not saying it's not doable but it's clunky. With Git you can resolve most of the changes. Another thing is that it's bit harder to prototype. Can you just throw away changes from some transport? I am not aware of this. You have to go all modified objects and manually revert changes and then delete transport.
It's definitely not broken and you can't have distributed VCS for ABAP but there is some space for improvement.
A change log (at the top) has value. I'll agree with that.
Nail hit on the head!
In most cases I find this as a mandate from the over-enthusiastic Client Manager. In my current project my Client PM wanted us to "put a statutory warning that the modified code is not really an acceptable fix (because we were only fixing an upgrade bug) and should there be a future cleanup, the approach needs to be re-looked". Well that's the short of it. I had write a whole essay of comments in the code. 😐
Great Blog and Discussion...
Now the Challenge in the Team: "How to motivate your colleagues to write better comments"?
Couldn't agree more with this blog in its entirety.
One rant is missing though, about blocks of commented code, even whole methods or subroutines (same argument about the last "controversial" rant applies: what the hell for do we have a versioning system in place!?!?)
Robert C. Martin, Clean Code, Heuristics G9.
First of all, nice topic.. It's a rather challenging process to understand how certain minds works (logically, or in many cases the opposite), however there are certain points you ought to understand:
- before a full code re-factor occurs, some methods could hold a few lines, which in turn makes it longer;
- a visual scan (eye) of source code, either in SE80 or Eclipse will prefer comments to normal ABAP syntax;
- this scan will make one find something in seconds rather than minutes or hours;
Having said that and seen what I've saw already (nobody here is a saint right?, we sometimes forget to look into our own bellybutton or our colleague's) -- how would you judge on that?
Sometimes, comments follow a line of thought or action a piece of code or a method is doing, and it serves as a quick reference to remind a developer what is the flow of that code.. and I would understand you getting back and saying: " but Daniel, if a method is doing more than it should, why not break in two? " -- sometimes, the re-factor cycle did not occur.
So when you look into those 'Useless Comments', not as a single unit but as a collection, they often would make sense to one or many..
But all in all, it is a nice topic, I can see this is from 2012 and I know better... "no one really does what one says", I once heard such and it's just.. precise.
PS: to the person above me about Dead Code, my bare eyes had to see 'IF 1 = 0" - unacceptable, by any standards no matter what excuse one would give..
Code inside IF 1 = 0 would be thinking, "I'm not even supposed to be here".
haha, troll code man..
Not really, that is done on purpose so the where used function for the message (which is in between the if endif statement) works. That is a good reason to do this. SAP uses this very often and I have had many times that I was grateful that this code was there.
So that one is definitely NOT useless code to my opinion.
That was pre MESSAGE...INTO code. If you are still using IF 1 = 2 to show the usage of the message then i would suggest you should rethink 🙂
I was also using 1 = 2, until a teammate questioned it.
There is a caveat though. Message..into gv_msgtext shows up in EPC checks, saying that gv_msgtext is not read anywhere.
I don't use it. I just say SAP uses it in standard code and it has proven to be very usefull.
Not all comments and dead code are useless.
The first extractor I wrote for BW was rejected at peer review because of exactly this way of referencing the message - it is part of the template!
Like I said. I never have used it myself but I have been able to solve more then just a few problems much faster because SAP used it in standard code and therefore I could locate the code where the message was raised easy and fast. And time is money in our world. 🙂
Exactly. Although SAP is a great software the older code is very, very, very bad.
Little to no encapsulation, business logic inside the UI logic, global variables everywhere, it's terrible.
MESSAGE ... INTO SY-MSGLI is a workaround as long as the ammendment MESSAGE ... NO-DISPLAY does not exist - when the MESSAGE statement is only supposed to fill the system message fields for a following log->add( ), like so:
message ... into sy-msgli.
log->add( ). " add() imports iv_msgid type symsgid default sy-msgid, etc.
By the way: You'll get no warning from the extended syntax check / code inspector when using sy-msgli, btw - as a remark for those who think good programming means to bring the code inspector down to 0 under any circumstances.
Of course, sy-msgli with its 70 characters is too short for most messages - it won't be used anyway, it's only a dummy as long as no NO-DISPLAY amendment to the MESSAGE statement exists (this could mean "forever", unfortunately).
The best option, however, would be to work with exception classes designed as (T100) message class: When creating the exception class, just check the checkbox "With message class", and in the "texts" tab specify the message id and assign the message variables to attributes of the exception class (btw, the CHAR50 restriction for message variables falls away for message exception classes, they can be typed with the proper type needed by the application). And the where-use for the T100-message will work.
* Example: T100- class mapped to message 00172
raise exception type zcx_no_transaction_authority
tcode = sy-tcode.
* ... and in the error handling code:
catch zcx_no_transaction_authority into data(lo_auth_ex).
log->add_from_exception( lo_auth_ex ).
That's the way i do it 🙂
If i cannot map the message to the text of the exception class i use the parameter TEXTID instead.
I am still using 1 = 0 (not 1 = 2, this one will get picked up extended check). I am open to re-think but you will have to convince me. I will try to play devil's advocate. ABAP compiler will automatically ignore code within block 1 = 0. In your case the code will actually get executed.
Just top be clear. I am not sure if ABAP compiler really eliminates dead code and I understand that running time difference is negligible. I just don't see any difference between
IF 1 = 0. MESSAGE ... ENDIF.
MESSAGE .... INTO ...
I guess this is one of the things where there is no way people will agree on one way. Same as with formatting option for pretty printer.
well.. the question is: SAP uses it, does that makes it right? --- search your heart before answering it..
PS: no sane person should code like SAP does, and I can get you over 100 function modules or even over 100 classes of PURE bad design.. lets all do better, because we end up having to maintain the code after all..
In standard SAP code, and don't tell me you don't have that in your system, it is used to be able to find where a message is raised . In which (part of) programs. If a problem has occurred in a production environment and you do a where used search in SE93 on the message raised then THE ONLY WAY it wil find it (In standard SAP code, and yes it might be old, but it will not change for the next years) is if there is the if 1=0,. statement.
You can then put a breakpoint on every instance and start debugging. Within 15 minutes you will probably have found the cause of the problem.
In any other way searching for the root cause of this problem it will take you much longer.
If you design your own program, you should document it properly where and when which message is raised and then you do not need to use the IF 1=0.
But to state that it is ALWAYS wrong is wrong by itself.
And I leave it with that. I really have nothing else to add to this and I haven't seen any alternatives in this discussion for this seraching of the places where a message is raised in (old) standard SAP code, although some people did claim that here are alternatives, but it seems they don't want to tell the rest of the community.
Instead of using the "Where-used", use watchpoints while debugging. Set the watchpoint for SY-MSGNO and you will probably get a hit in no time.
Unless I misunderstood the situation.
It will work too but it wont work as fast as the where used.
In production environment time is money and I have been praising SAP for having the IF 1=0 statement many times and so did my clients.
Of course there are better ways, but we all have to deal with the code SAP delivers and lots of these code is not going to change in the near future (Like FI or eben more SD, where almost the whole module is in one program).
I agree that in new code there are other ways to deal with it. But has any one of you who disagrees rewritten the SAP SD module ? If so, OK many praises to you. If not then the where used in combination with IF 1=0. is definitely not useless or dead code.
I am simply stating that this statement is not ALWAYS useless like some people are saying.
You are inside the transaction and get the error:
It's much faster then "Where-used" you don't even have to leave the transaction, or check which of the 50 "where-used" hits is the right one.
True you are right, the only difference is that the message has already been raised with your method and I might want to know the situation just before the message is raised. So I am a little bit more flexible where I want to have the program stop because after I do the where used I can choose to set the breakpoint a few lines before the message will be raised to see the state of an object.
But you are right. I stand corrected 🙂
I also do that, after the watchpoint is reached I look at the code, maybe at the call stack, and put a breakpoint (and run the error again). The only diference is that I put the breakpoint in the debugger while you do it after using the where-list.
In my view, the way I do it is still faster because:
As a con, the debugger is usually slower to scroll.
You are right again. I will start using your method next time I am asked to check such a problem. Thanks.
Although I am nowadays more involved as PI developer I still do some ABAP work sometimes, especially helping out to solve problems in production.
I guess since the where used method was doing fine for me I never looked any further for other ways to solve the problem.
So the use of IF 1=0, is now not as important anymore to me as it was before this discussion. Just as comments can be useful in analysing and understanding standard SAP code this IF 1=0 will still have some use (although little) to know which message will be raised at that point.
In my understanding watchpoints only work for the program they are set for. So, if your program calls a function that then sets the message the watchpoint won't trigger as it would have occurred inside another program, namely the program for the function-group.
Ultimately, it is more painful than a where-used, don't you think?
The "IF 1=0" is ugly, but I haven't seen a better alternative described in this thread.
This is not the case. Sy-msgno watchpoints trigger at the location they are set. You may need to switch on system debugging. I know this for sure, because I've done it hundreds of times.
It is in fact more useful and less painful than "where-used", because it gives you the full context of the place where the error is. Anyway, I don't understand your objection - surely if you find the location via where-used, the next step is to set a break-point at that spot?
it's always open for discussion which comment gives additional value and which one is useless. I agree that evolution of code does not help. But sometimes it's a clear cut. Comments in "Create context object" should not be in the code. Also you work within a team and trying to force your style on others and acting like you are something better does not help the project.
Regarding 1 = 0. Not sure why this is really irritating for you. As others mentioned it's for where-used list. SAP has even an exception in extended check for this. So it will not complain about it. I guess you do not mind doxygen comments in C++ or Java code. Basically same purpose. I definitely do not classify it as a dead code.
Thanks for your answer...
Usually senior developers would not 'force' junior developers but rather show them the way and alternatives on how to get stuff done.. It's not about forcing.. it's about teaching. Seniors would often show ways and explain why such method or technique, and of course.. its up to the other side make use or not of such.
Regarding "1 = 0", I have no problem with 1 = 0 but I do have a lot of issues with "IF 1 = 0" and mostly, because there are other and more elegant ways of doing such.. people often are too lazy to research... as example, at this project I'm currently working people are so 'not up-to-date' that they still use 'concatenate', 'add ', 'create object' (no, it's not a dynamic type) - and we are running the latest version of abap... for instance, they could simply make a comment and search in eclipse, no "IF 1 = 0" required. (and look, it's not a useless comment..)
And " IF 1 = 0" is just the tip of the iceberg, because people who actually does such will have "no mercy for kittens".. I mean, " IF 1 = 0 " is not even like, "that bad" considering I've seen lately a "zcl_address=>map_s ex()" method.. I wished I had a fork nearby to just stab my eyes and put an end on it, but no.. no sharp objects nearby!
All I want to say is, the same people who is capable of doing " IF 1 = 0 " is capable of doing such methods I mentioned above, and that's why I think we should stop bad habits or bad techniques and not use ' but SAP uses it ' as an excuse, because that is no excuse for plain bad code..
please stop playing this charade. To be clear we are on the same project and you are criticizing my code. I am OK with this but please be clear about it.
Regarding 1 = 0. You still have not stated a better way to be able to flag spot where you raise a message. Based on my experience it's invaluable to use where-used functionality when you have code in production and you are getting an error message. Somebody else mentioned here that you can do INTO variable. I personally don't see the difference between these two approaches. Does anyone have a better approach for this?
Also keeping all your findings for yourself while you spend time reading others code is definitely not going to improve situation.
As mentioned on my previous response, a text search would suffice.. and it seems clear to me you took some of my comments far more seriously they were supposed to be.
It's really hard to judge a comment if it's full of double meanings and you don't know person very well.
If you mean search text in source code then IMO it's not sufficient. It's OK for your own code. But it seems like you've never worked with SAP code base. You get these error messages that are hidden deeply in guts of SAP. You have no idea where to search and you can't just put a break point on ABAP statement MESSAGE. It's invaluable just to search where message is raised, put a break point there and re-run test case. I still haven't heard argument why it shouldn't be done. Calling it ugly is not an argument. The purpose of code is to be executed and provide some value to user who runs it. If I can make a code easier to debug and then save some debugging time in future then it seems to be a right thing to do.
I just might have convinced myself to use MESSAGE INTO variant instead of 1 = 0 variant because with the first one you can put a break point on ABAP statement MESSAGE and it will stop there. This might be really handy in some cases.
Exactly my point.
Why don't you use watchpoints instead of breakpoints? Just watch for a change in SY-MSGNO to the number of the message you are looking for.
You don't need MESSAGE INTO or IF 1=0.
thanks for the input. I don't agree that you don't need MESSAGE INTO or IF 1 = 0. Sometimes I just have an application log and I can't just simply re-run program.
But I think I am converted. Now I think it's better to use MESSAGE INTO variant. The reasons for this is that it's better that statement MESSAGE is actually executed. Where used still works and you case use debugger to get there if possible. There is still chance that executing message is not desired but I can't think of any case right now.
It's cool to be up-to-date, but then you need to reuse your program in an old SAP system, and there goes all the coolness....
Add is very old, but not using CREATE OBJECT is a bit dangerous....
I am usually more concerned if people don't use ABAP Objects, because that's a programming paradigm, which is much more important that using an old fashioned keyword.
As far I can see, case one of your Customers does a lot of investments in bringing the abap stack into the latest version and as a Consultant you don't deliver up to that standard, this is actually an issue.. also, it's not a nice thing to limit yourself or any code you developed based on the 'lowest common denominator' - I mean, it doesn't seem logical or even fair to say " .. I kinda don't use new additions because we have another Customer running 4.6c and we may want to re-use code of a given Customer to another .. "
Also, since 7.4 there is no more need to use the verbosity 'create object', instead just a 'new' suffice.. and this is actually how, let's say, most languages deal with the problem.
About abap objects, I'm usually more concerned with people who actually uses it but have no clue what "object orientation" really is, so you end up with all these static classes or methods which has no meaning, no hierarchy,no state and no real responsibility in the overall design of a solution or code-base.. bugs me.
Like I said, that is stretching it, those versions are not supported by SAP anymore and realistically no customer keeps its abap stack updated for the new ABAP additions. They don't care.
The fact that it is less verbose is better for the developers that needs to write less code, not for the person that does the maintenance and much less for the customer that will never look at the code and will have a harder time finding ABAPers that actually understand the new additions.
That is not using ABAP Objects, that is called using SE24. Not the same thing.
I beg to differ. Less verbose code (if* properly constructed!) should be easier to read, and therefore easier and quicker (and therefore cheaper) to maintain (if* you have properly trained and skilled developers!). Customers care about this as they are paying for the maintenance.
The biggest issue with ABAP objects as the start was the verbosity. It made it hard to read and follow. That's improved greatly with the function type method calls. New is a continuation of this, and a good thing in my book. Trouble is you always get idiots who'll use the shorter forms to make their code unreadable (write once, read never). So, where you can write things more neatly and clearly, definitely do so, but always concentrate on clarity over compactness.
*Yes, I know, those two "ifs" are enormous, sad to say. 🙁
Should. But is it? My point is that most ABAPers out there, probably the ones doing maintenance:
I know for a fact, that it's a much greater headache for the customer if you use up-to-date technology. I'm not even going to mention SAPUI5....
Like you said, those are two enormous IFs that in the real world fail to evaluate true in most places.
I use ABAP Objects and Webdynpro because they are development paradigms that not only decrease development time, but also my maintenance time and that by far compensates the inability of some people to read it. The benefits of new additions are marginal at best.
Customers benefit from a common, already tested code base, much more then they benefit from new ABAP additions. Besides, it's the same as developers not targeting the latest Android API when they are choosing where to put their investment. If I develop a product, I'm going to develop it in a technology that gives me a wider customer base unless the extra functionality is worth it.
the question I throw in the room. Why do the developer-management of these companies doesn't take care about using this advanced techniques?
Perhaps because they are thinking some of your mentioned here is obsolete too? Just in my mind, Webdynpro...
Why they aren't using OO?
Because the argument is, that it isn't that readable. Again, the management doesn't take care about it?
New additions to ABAP?
Because they aren't that interested and nobody is telling them this things?
I know, it's up to everybody of us doing research and get involved with the new things. But I'm pretty sure, that there are also a lot of people out doesn't get the time for it or even are not that interested in these things, because the management doesn't care about it.
So, the fish stinks at the head first.
Because most of the time, they are middle level managers that don't know SAP technology. At most they were functional consultants, it's extremely rare (at least from my experience) to see a manager with a "technical" background.
I force my developers to use ABAP Objects (in a object oriented way) and Webdynpro because I know the benefits first hand. For a less-technical manager, it goes like this:
- Manager: Hey people I heard of ABAP Objects and Webdynpro, are we using it in our developments? People tell me it's good and modern.
- Developer: Nah, it's all a bunch of work. Plain ABAP works just fine for what we do.
- Manager: Ok.
How do you force people to learn new things, if you don't know what those things are in detail? You don't because you lose the argument.
That hits the point. The big question is, if SAP itself should force trainings for manager and give them arguments on the hand, why to use it and tell them also, that the "more" time is a very good investment in the future of the lifetime (source and also in the company).
Thank you for bringing that to the paper. I'm happy to see, that are more people thinking in my way out there.
I don't think they really care what their customers use in their custom developments. I've never seen a SAP rep be a true partner to his customer, all I've seen is sales pushing. If it weren't for the consultants that keep the customers happy, and are their true partners, SAP would have had to change their ways a long time ago (or go out of business).
In addition to my previous post, I have to say that senior developers also don't help most of the time. There are a lot of ABAP dinossaurs out there, that use SE51 to make complex GUIs also don't want to learn, and want to keep doing the same thing forever. Juniors that come into their teams will only learn SE51, because that what they were taught.
I placed the blame on technical ignorance of the managers, but that ignores part of the problem, that comes from senior ABAPers themselves.
That's the message that needs to be pushed.
SAP are helping in a little way. PERFORMs are now labelled obsolete in later versions. But we still have newbies being advised to use ON CHANGE, tables with header lines, BINARY SEARCH instead of SORTED or HASHED tables, or FAE etc. so I'm not hopeful.
At the moment there are too many stupid programmers regurgitating the same old nonsense to the next intake - partly this is because ABAP is often the entry level into the industry. After two years, people want out; either to management or to functional areas, where they get more money and kudos. So you end up with developers who've either got less than two year's experience, or weren't good enough to shift. And endless cycle of craptitude.
That's the main problem. Like I said in another post, the worse problem in the SAP world is it's attitude towards developers, one that is cultural and deeply rooted.
When I came into the SAP world 10 years ago (I'm a electrical engineer), one of the strangest things was seeing functional consultants write pseudo code, with tables and fields, and then give it to a ABAPer to code the SELECT. Crazy stuff. If they write all the pseudo code with the technical objects, why not make the SELECT? It's easy right? Wrong.
I found that the problem was that functional consultant were most of the times from management and economics degrees and thought the programming aspect was a lesser work, which was better left for those other guys (sorry, my opinion). Which of course must be told what to do, and have a lesser wage (because it's lesser work after all...). Configuring SPRO is much more important and "cool" right? ....
I saw the attitude change in my company when more and more engineers and computer science graduates became consultants (period), people who didn't mind programming when it was needed to get things done, and started pushing for a change in the mindset. Nowadays, consultants (period) who also do programming are highly valued (but are still rare, because many believe the cheap ABAP is the way to go, I disagree).
if you have some spare time watch this interesting talk by Fred George. He describes a company where they got rid of business analysts/functionals and dedicated QA engineers and the company is run by programmers. I found that quite interesting. I don't think that it would work in world of commercial of the shelf but still interesting.
BTW another talk about micro services from same conference and same author is interesting as well.
I watched it, here is my comment: Rüdiger Plantiko – Google+ - Leaving out the roles like Business analyst, project…
Yeah, your are right again. SAP should be that partner. I mean, just for being not the partner in the past is not a argument not being the partner nowadays. IT's up to us to tell this story here.
And of course, all the juniors got a senior in front showing SE38 and not AIE are also part of this.
In the end the management has to give the initial shot and that is in my opinion SAP itself.
I might try to talk about it and see if there might be a small little step forward.
Thanks for the conversation.
Thank you, really appreciate it.
This is a very interesting topic, more demand less offer + lack of expertise in the customer side = no healthy competence at all.
IMHO I don't see a medium term solution for this, unless SAP start to audit projects, but I'm quite sceptical about this because SAP can't fight againts all the partners, partner happy = more licences sold, and everybody knows, bigger partner = harder to control the expertise and the willing to keep being and expert of the professionals, but more chances to sell licences. 😈
Anyway I wonder what will happens if you really have to fight for a position and not wait until you recieve x linkedin job openings every week. 😈
The problem with SAP audits is the required quality of the SAP employee who does the audit. They actually have to know SAP and SAP Guidelines better then the consultants....
I see your point, and let me be a little nasty 😈 , would you audit my project? uhhh excuse me who are you?
I don't get your comment. I wasn't saying I should audit your project, I just stated a basic fact.
Yup, maybe I wasn't clear enough, I understood your point, but is also a basic fact a reviewer will be a human or a program coded by a human. I assume if someone from SAP comes to my project to audit it, will be someone with all the guidelines, standards, blah blah blah tattooed, if this doesn't happen, as we say in Spain "Shut down and let's go to somwhere else".
Indeed. Once during an internal audit, we got a fail because I'd (apparently) not followed procedures. In fact I was the one responsible for defining the procedures, the procedure previously in place was flawed, so I'd replaced it with a new better way of doing things, which, this being a validated system, I properly documented.
The auditor wasn't aware of the new procedures... and he was the one who'd come up with the flawed one. It all got a bit political... 😀
Hi Matthew, I totally know what you are talking about... Kind a hard thing to discuss and a lot of unnecessary work to the who have to write the statements about the review...
Not my favorite job...
Hello Joao Sousa,
I had to branch out the conversation because of comment box gets narrower as the # of comments increase.
Afaik you cannot trace the message with a watchpoint on SY-MSGID, SY-MSGNO if it cannot be referenced by the "where-used".
Please run the following code snippet in your system, you should get an application log like this -
Let me know which messages you can trace using the watchpoint on the relevant SYST fields.
I know that MESSAGE INTO... gives you SLIN message & i don't use SY-MSGLI because it is marked as obsolete. IMHO the best way to use it is to create an exception class & map it to a text.
Let me know if i'm missing something.
I didn't say it "catched them all" 🙂
Watchpoints don't work with those because they don't set the sy-msgno variable. One of the most annoying ones is VL01N that has this kind of log.
The MIGO (rather complex tcode with lots of screen handling and passing data) developers solved this via their own special debug console.
Type MIGO_DEBUG into the ok-code and more menus become available from code which is otherwise dead (and also appears never to have been finished...) because they seem to also have discovered watchpoints and layered debugging.
Perhaps we can ask SAP for a customer system field for messages (which also does not dump if type is not known?) which is optionally always there and developers can write their doubts into, should they need it oneday.
Just a thought..
IMO 1 = 0 assertions should not be transported to production.
because you knew everything he wrote and never made useless comments in your code?
I hate the commented section most. I mean, instead of working thoruhg 500 lines of useless coding I would prefer some words, why that stuff was deleted with a clue to a document (In best case as ABAP-Docu attached in the new build class 😎 )