Technical Articles
Good Constant / Bad Constant
Hi Folks,
I think I can get fairly rapid consensus on this here, but I run into this so often I thought I will bring this up for “discussion” again. I tried searching around and while there has been a lot of generic discussions, I haven’t seen many robust examples yet. I have also seen so many code reviewers missing the point of the existing discussions and SAP documentation (Literals) – I want to see if I can word this question / discussion point in such a way that it provides enough examples – good and bad – about how constants should be used and when literals are alright.
First here is some real code from a productive system (I have obfuscated some of the code)…
CONSTANTS: c_enhname TYPE z(obfuscated) VALUE 'OBFUSCATED',
c_b TYPE c VALUE 'B',
c_e TYPE c VALUE 'E',
c_n TYPE c VALUE 'N',
c_x TYPE c VALUE 'X',
c_y TYPE c VALUE 'Y',
c_08(2) TYPE c VALUE '08',
c_12(2) TYPE c VALUE '12',
c_261(3) TYPE c VALUE '261'.
...
" Example 1
FOR ALL ENTRIES IN lt_vekp
WHERE handle = lt_vekp-handle AND
( object = '08' OR object = '12' ).
...
" Example 2
SELECT * FROM (ztable) "Obfuscated for Public consumption
INTO TABLE lt_zrf
WHERE aufnr = process_order AND
( status = c_y OR status = c_b ) AND
zflag = ''.
...
" Example 3
SELECT (obfuscated field list)
FROM resb INTO TABLE lt_resb
WHERE aufnr = process_order AND
bwart = c_261. I think the statement about declaring constants with meaningful names will not be met with much resistance. In example 1, the object ’08’ and ’12’ are used several times in this code. The fact that C_08 and C_12 mean nothing and the code has not been commented anywhere means that no one knows what this means at all!
In this case especially since the constants are used in several other places I would absolutely recommend that the programmer use a properly named constant like below.
CONSTANTS:
c_work_order_link(2) TYPE c VALUE '08',
c_unused_handling_unit(2) TYPE c VALUE '12'.
...
" Example 1
FOR ALL ENTRIES IN lt_vekp
WHERE handle = lt_vekp-handle AND
( object = c_work_order_link OR object = c_unused_handling_unit ).In example 2, the constants are used only in 1 place in the entire code. Frankly, I don’t know what the Y and B mean any more, I just found this in an old email – I guess that is a perfect example of poorly documented code. In this case – queue first debatable point – I argue that there are 2 ways that are equally good. There are no real benefits either way if you know this code is never going to use that constant ever again.
CONSTANTS:
c_baffled TYPE c VALUE 'B',
c_yoyoing TYPE c VALUE 'Y'.
" Example 2
SELECT * FROM (ztable) "Obfuscated for Public consumption
INTO TABLE lt_zrf
WHERE aufnr = process_order AND
( status = c_yoyoing OR status = c_baffled ) AND
zflag = ''.
"... OR...
SELECT * FROM (ztable) "Obfuscated for Public consumption
INTO TABLE lt_zrf
WHERE aufnr = process_order AND
( status = 'Y' OR status = 'B' ) AND "Yoyoing or Baffled
zflag = ''.The 3rd example, I believe is similar to the above – but is closer to an example of where the semantic meaning is pretty obvious to anyone who knows enough to deal with goods issues to an order. Pretty much everyone in this space would know what a 261 movement is. Of course, one could argue that a finance centric ABAP’er might be poking around and not know what a 261 is – I would argue back at that point that telling them it is a goods issue doesn’t mean much more to that person without providing a lot more context to them.
The last and most egregious aggravation is the C_X. The same constant is then used in several places in the program to mean True, Deleted, Empty and Used up. I think this is worst example of constant usage. Though using a literals in multiple places is not the best practice, I would argue that literal usage is the lesser of two evils. Of course, the best approach would be to have separate constants for each usage.
CONSTANTS:
c_true TYPE c VALUE 'X',
c_deleted TYPE c VALUE 'X',
c_empty TYPE c VALUE 'X',
c_used_up TYPE c VALUE 'X'.This way, the code in individual places is made more readable. In future, for what ever reason, if one of those constants has to change, then just the one can be changed without effecting any of the other code. For instance if one of the tables grows to have additional meanings on the flag, and C_EMPTY becomes ‘E’ and the ‘X’ in that case comes to mean C_DO_NOT_USE. You will be able to make that change without changing all the other constants.
Note: I know there have been several wonderful discussions about naming conventions and the use of classes / interfaces for constants. I have a feeling that somebody will try to bring that up in the comments – I am going to try to pre-empt that by saying, I believe those are different discussions of equal importance, but not what I am trying to discuss here.
Interesting blog. I don’t see a problem with literals where the use is obvious:
Likewise, a one-off use is also no issue. I detest the use of 'literal constants' like c_08, “just because our development standards say so”.
A modern development approach is to write simple and refactor, which development tools such as Eclipse facilitate very well. (I particularly like how converting a literal into a constant can automatically replace all instances of the literal in the current code).
So I am quite happy to write
If at a later stage we identify that it is best suited as a constant (e.g. when the number needs to change, or when I need to re-use the same value somewhere else), Eclipse pretty much autmates conversion: <ctrl>-F1, extract constant, give it a name, done.
So my constants evolve. They start out as literals, then local constants, then maybe class constants, or move to local classes or interfaces of their own. I also like using structures to group related constants.
Some may disagree, but this is also the one time where I am happy to drop the ZIF_/LIF_ naming convention for interfaces, just to underline that this is a pure constants interface.
So I may end up with:
The reason I’d use constant c_cancelled rather than literal ‘CANCELLED’ is that if I need to use it more than once, I prevent errors like:
IF blah EQ ‘CANCELED’.
So, no, it doesn’t make things any clearer, but it does make your program slightly more robust. "I'll only ever use it once" is often heard... and equally often violated!
Not just because of potential typos, but I’m not much in favor of hard-coded literals or constants with a value like ‘CANCELLED’ in general as these are language-dependent but cannot be translated if such a text – instead of e.g. a numeric or alpha-numeric counterpart (with texts controlled via the value-range of a domain). – is used in the data
I don’t agree, constant names are even less translatable and English is the de-facto language of computing.
An english worded config value is easier to understand when scrolling through data in SE16 than a bunch of two or three-digit numerics or character values that are a throwback to the days when values were stored in full and DB space was at a premium.
For this reason I’m always in favour of descriptive English values when creating configuration-type values. They should also be translated if applicable. People whose native language is not English will always have better odds of understanding ‘CANCELLED’ than ‘048’ without resorting to F4, so you can only win.
That's exactly why I use
rather than
one less click to know the text.
Slightly off topic, but I was just looking at some old code where I had used alphabetic text element names that could be reused in code with the meaning being obvious...
I think you misunderstood my comment, because I agree with your reason and said as much. I may write a piece of code the first time round using a literal. If I need to use it again I refactor into a constant. This may be 5 minutes or 5 days later or not at all. Or I may refactor into a select from a config table.
As a side note, a typo in a constant declaration is just as easy 🙂 From personal experience these are harder to track down as we tend to assume constant = correct.
This is why I've also pondered quite some time whether constants should be tested.
I did get that (I think!). My point is rather that I’ll do the constant at the first use.
Constants like c_101 which has a value of 202 are a little annoying... ?
Perhaps it's just a matter of working style. I like to work from the inside out, write things as simple as possible to get it working and then flesh it out. A constant at the initial writing stage is an unnecessary layer of complexity.
e.g. Sometimes the values need to change a few times before it's correct. I might start off with a known good value to ensure the basics work. So for me it's better to have it there in a literal as it's less jumping back and forth. And during the cleanup phase I sometimes also convert to constants just for clarity.
A lot of old rules are being eroded by modern editors, I don't see the constant/literal debate as important as it once was because it's so easy to refactor in a few seconds once you add that second usage.
Don't you know if you drink enough and look at the C_101 hard enough, it will double up and equal 202? 😛
I'm with you on this, Mike, and usually go through the similar "evolution" process with the constants.
In my old job, we used a consulting company for some ABAP work and those folks had "always use constants" rule in their guidelines (there was no special rule in ours). Sadly, this resulted exactly in the bad examples shown in this blog and in ABAP Gore. They'd create a constant even if it's used once and call it C_WE value 'WE'. 🙁
I suspect that the real purpose of this rule was to use constants with descriptive names, like c_ship_to instead of c_we. But somehow is was not communicated well.
It's a typical example of "because we've always done it like that".
These things made sense a long time ago. Refactoring was hard, especially in the old line-based editor. Searching through multiple programs was nigh on impossible. Variables and constants were declared up front partly because it made it easier to gauge the memory consumption. And program modules with 1000+ lines were almost the norm, which made it hard to traverse code to find that literal, so having everything declared in one place was also a time saver.
But that was last century... 🙂
I find sometimes, the value of an object is more meaningful to SAP-versed people than a description. For example, movement type 101. Anyone spending any time in MM will know this is Goods Receipt for (purchase) order.
c_gr_for_po type bwart value '101'.
may well be perceived as less meaningful, than just c_mvmt_101. For that reason, I'd use c_101_gr_for_po. This way you've got the well known shorthand, and the meaning.
If I have many movement types in a program, I might consider.
agreed , given the way in which our fronted moving toward JS i wish ABAP had been case sensitive and we could have used camel case or something instead of this bit irritating:) just a thought. Sometime like below looks more readable and less documentation.
Is it possible to configure ADT so that constants are a specific colour?
yes to an extent just found out:) https://blogs.sap.com/2016/04/01/customize-syntax-coloring-in-abap-in-eclipse-not-an-april-joke/ .
I think you meant to say c_gr_for_po would be perceived as more meaningful, than just c_mvmt_101.
I completely agree with this, and if I were to use a constant in this situation I would name it either like your last example or most likely because my custom programs usually address only one thing at a time I would use something like c_mvmt_gr_for_po.
However, practically, this is one of the few places I would second Mike Pokrakaa's sentiment above and just use the literal.
My criteria for using the literal would be along the lines of 1) is it commonly understood in our world 2) is the meaning never going to change in our world (if 101 suddenly meant goods issue we are all in trouble!) 3) is hard-coding ok (applicable for either constant or literal.
When in doubt I would think that decent documentation of a literal would beat out a semi-descriptive constant name. For instance, I could argue if you are accounting for folks that don't know what a '101' move is, they wouldn't know if 'GR' stood for 'Good Receipt' or 'Goods Removal' either. And is the PO a purchase or production order? I think the following is even more descriptive.
Not just that, if I am calling a custom function someone decides to change the data type on the function module interface, I am not stuck changing this program too. I've had that *never ever* happen to me before 🙂
Interesting conversation so far.
My opinion over constants has changed over the years. I think I would nowadays object strongly to the first development standard document that I wrote.
My view on this point can be distilled by the sentence 'Does this add value?'.
So I stopped using c_true, I just use 'X'. And in other cases I try to see if it adds anything in a debugging session seen by a techno/functional consultant, I also try to not abbreviate and use longer constant declarations.
+1 - Rob, I like this message. A lot of folks I deal with think the use of even a single literal is evil but can't explain why. I think that is how they end up with the C_X constants.
They need to hear this message! -> ‘Does this add value?'
Another important advantage in using constants is the where-used function.
And although you can always use the Find function, like Mike Pokrakaa has suggested, it may lead to irrelevant results and/or not cover the same scope (especially for global/cross-report constants).
Good point.
I assume the find function you referred to is my comment about Eclipse finding multiple occurrences. I didn't really suggest this as a tool. This is just in the local scope and more of a convenient shortcut for refactoring usages you know about than a way of finding them. We're better off doing a manual search the old fashioned way if we need to do the latter.
Example: A method may contain two different SELECTs with a literal. In eclipse I can put my cursor on the literal, do an 'extract constant' quick fix, and overwrite the proposed name. It will automatically change another literal containing the same text in the same code block, nothing else.
How about abap_bool TYPE (abap_true & abap_false constants) - good or bad? 🙂
I definitely use abap_true / abap_false. The meaning is self evident and not going to change – and you don’t even have to explicitly declare it. To boot, if you are counting, it will save you 1 Byte of memory not having to re-declare a variable – so, when we go back to counting memory in KB your program will win
Two very enthusiastic thumbs up for abap_true and abap_false. In fact, I'm working on an interface right now where I purposely created methods to translate all the "custom boolean" values (like Y/N) from DB strictly into abap_true/false. This makes the program much easier to understand IMHO.