Hardcoding

Development (ABAP Development WorkBench, ABAP/4 programming)

Moderators: Snowy, thx4allthefish, YuriT, Gothmog

YuriT
Posts: 885
Joined: Fri Feb 03, 2006 6:40 am
Location: Basel/Riga

Hardcoding

Post by YuriT » Fri Oct 04, 2013 1:44 am

Hi, guys

I am trying to put my thoughts together regarding hard-coding.

My main points are:

1) Avoiding ANY hard-coding is silly. Nothing wrong with MOVE 'X' to p_checkbox. More then that, CONSTANT c_x TYPE c VALUE 'X' freaks me out. Same goes for CONSTANTS c_seventytwo TYPE i VALUE 72.
2) Putting hard-coded values to constants only solves half of the problem. Easier to maintain within the code, but It is still hard-coding, because you still need to modify the code.
3) If you really into making it flexible, use customizing tables. Create your own or reuse standard SAP approaches like TVARVC. Until you do, it still counts as being hard-coded no matter how many constants you create.

I would really appreciate your thoughts on this topic and whether you agree with things I stated above.

Baz
Posts: 4748
Joined: Fri Nov 08, 2002 5:54 am
Location: He's out there! somewhere!!!!
Contact:

Re: Hardcoding

Post by Baz » Fri Oct 04, 2013 3:01 am

I try and use a table to store these values wherever is practical. otherwise, why bother setting a constant with a value.
you're just adding extra code! unless you are being paid for code.

Once i knew there were going to be several programs with similar/same values so i created an include with these values and used that in the Function Modules.
Baz

AsPiRiNg tUlY iDiOt Image

http://www.catb.org/~esr/faqs/smart-questions.html

Image

check out my Podcasts http://dj-baz.podomatic.com

Sharpshooter
Posts: 1172
Joined: Wed Mar 17, 2010 12:01 pm
Location: In the dark

Re: Hardcoding

Post by Sharpshooter » Fri Oct 04, 2013 6:39 am

I would generally agree on all points.
However see Rich's topic in the knowledge corner for another perspective on where using constants is helpful:
viewtopic.php?f=31&t=326521


Dave
Good luck!

Grogan
Posts: 365
Joined: Thu Aug 04, 2005 7:16 pm
Location: Australia

Re: Hardcoding

Post by Grogan » Sat Oct 05, 2013 5:39 pm

And what is the big difference between

Code: Select all

select  * from t5ndk into table i_t5ndk where bunum eq c_init_bunum.
as opposed to

Code: Select all

select  * from t5ndk into table i_t5ndk where bunum is initial
?
Grogan

Award cash if useful.

Rich
Posts: 7116
Joined: Thu Oct 31, 2002 4:47 pm
Location: Liverpool
Contact:

Re: Hardcoding

Post by Rich » Thu Oct 10, 2013 10:22 am

Yuri,

You know what I think already.

So, for things like company codes that are to be processed, etc etc (ie things that may change throughout the programs life) should go into a customising table.

Magic strings and magic numbers should be placed into named constants at the relevant scoping level - form, module, global. The name should reflect the meaning of the constant, not what it remains.

C_X gives me the hives. C_TRUE is more like it....

I tend to strive for readability. The easier a program is to read, the easier it is to understand.
Regards

Rich

Image
Abap KC:http://www.richard-harper.me.uk/Kb
SFMDR:http://www.se37.com

YuriT
Posts: 885
Joined: Fri Feb 03, 2006 6:40 am
Location: Basel/Riga

Re: Hardcoding

Post by YuriT » Fri Oct 11, 2013 1:53 am

Great point about types. I guess my main point is that you shouldn't just blindly create a constant for anything in single quotes. Think WHY constants are needed in the first place. Filed names in field catalog is another good example.

I am still a bit skeptical about c_true. 'X' looks really self-explanatory to me. Also c_true will not work with LOOP AT SCREEN.

YuriT
Posts: 885
Joined: Fri Feb 03, 2006 6:40 am
Location: Basel/Riga

Re: Hardcoding

Post by YuriT » Fri Oct 11, 2013 2:06 am

And what is the big difference between

Code: Select all

select  * from t5ndk into table i_t5ndk where bunum eq c_init_bunum.
as opposed to

Code: Select all

select  * from t5ndk into table i_t5ndk where bunum is initial
?

Rich
Posts: 7116
Joined: Thu Oct 31, 2002 4:47 pm
Location: Liverpool
Contact:

Re: Hardcoding

Post by Rich » Tue Oct 15, 2013 2:29 am

YuriT wrote:Great point about types. I guess my main point is that you shouldn't just blindly create a constant for anything in single quotes. Think WHY constants are needed in the first place. Filed names in field catalog is another good example.

I am still a bit skeptical about c_true. 'X' looks really self-explanatory to me. Also c_true will not work with LOOP AT SCREEN.
But then the screen values are not blank or 'X', they are 1 or 0 and then I call them c_On or c_Off.... 8)
Regards

Rich

Image
Abap KC:http://www.richard-harper.me.uk/Kb
SFMDR:http://www.se37.com

Baz
Posts: 4748
Joined: Fri Nov 08, 2002 5:54 am
Location: He's out there! somewhere!!!!
Contact:

Re: Hardcoding

Post by Baz » Fri Oct 25, 2013 4:09 am

i am on a project where the company has split. the new one takes a backup of the system and uses it as their instance...

they want a new controlling area, chart of accounts, company code, cost element, cost centre structure....

my job is to see if their is any hardcoding...... so far i have identified over 1200+ custom Z Tables. 200 are config tables that hold company code and chart of accounts... over 1200+ z transactions....


here are some of the highlights i have found so far... (This makes me feel soooo much better about my code)...


Code: Select all

types: begin of ty_data,
         lname               type char50,
         fname               type char50,
         uname               type char50,
         ukost               type char50,
         usite               type char50,
         uloca               type char50,
         umana               type char50,
         umail               type char50,
         upern               type char50,
         mname               type char50,
         mmail               type char50,
         mpern               type char50,
         rname               type char50,
         rmail               type char50,
         rpern               type char50,
         ttype               type char50,
         atype               type char50,
         tcard               type char50,
         batch               type char50,
         bukrs               type char50,
         kostl               type char50,
         cbcan               type char50,
         rdate               type char50,
         adate               type char50,
         imeth               type char50,
         idate               type char50,
         cstat               type char50,
         mlimi               type char50,
         climi               type char50,
         tcexp               type char50,
         tccan               type char50,
         tlimi               type char50,
         tlvab               type char50,
         tlvbi               type char50,
         notes               type char50,
       end of ty_data.
WTF????? BUKRS and KOSTL Char 50.... okay....

Code: Select all

SELECT SINGLE * FROM marv
    WHERE bukrs = likp-vkorg.
hmmmmm dubious... especially when there is more than sales org but only one company code....

Code: Select all

SINGLE * FROM zfi_def INTO gs_default WHERE company = '0100' .

Code: Select all

SELECT * FROM skat
           INTO TABLE ct_skat
           WHERE  spras   = sy-langu
           AND    ktopl   = 'TILC'

Code: Select all

SELECT prctr zpcattid04 INTO TABLE gt_pcatt4
    FROM zreorgpcassign
   WHERE kokrs EQ '8000'
     AND datbi GE sy-datum
     AND datab LE sy-datum
   ORDER BY prctr.

Code: Select all

SELECT * FROM csks
            WHERE kostl EQ cost_center
              AND kokrs EQ '0001'.
admittedly some of this code goes back 12 years... The client thought there was only a little so only wanted me a day to analyse...
with the amount i have found so far, it could be a few weeks. and then the fun of changing it all! but thankfully i know better than to use hardcoded values.....
Baz

AsPiRiNg tUlY iDiOt Image

http://www.catb.org/~esr/faqs/smart-questions.html

Image

check out my Podcasts http://dj-baz.podomatic.com

Grogan
Posts: 365
Joined: Thu Aug 04, 2005 7:16 pm
Location: Australia

Re: Hardcoding

Post by Grogan » Sat Oct 26, 2013 9:23 pm

Great topic, YuriT !

I can empathise with Baz having been through a similar exercise myself in a previous lifetime. Amongst other tasks I recall having to write a program to dynamically scan hundreds of custom Z* tables looking for occurrences of the old company name.

Anyway, consider a piece of code that is a candidate for using a constant:

Code: Select all

IF bukrs EQ '0100'.
*   Special logic.  Add custom ZWIDGET segment to IDoc output for Acme Corp only.
ENDIF.
This logic could be written by the programmer as:-

1) Hard coded, as shown above.
Bad. Do. Not. Do. This.

2) Program constant with meaningless name.

Code: Select all

CONSTANTS c_cocd_0100        TYPE bukrs value '0100'.
IF bukrs EQ c_cocd_0100...
Slightly less bad. Technically a constant. If you have found the program then you can do a where-used list on the value.
But no easy way to find the program. Provides no meaning and, even worse, still gives Rich hives.

3) Program constant with meaningful name.

Code: Select all

CONSTANTS c_cocd_acme        TYPE bukrs value '0100'.
IF bukrs EQ c_cocd_acme...
OK, but still not great: at least it adds meaning to the reader and should show up in source code string-scan for the company name.

4) Class or Interface Constant.

Code: Select all

IF bukrs EQ zif_company_constants=>c_cocd_acme...
Better: now we have repository object that allows us to do a where-used list for every program, class or web-dynpro that references the value.

5) Class or Interface Method.

Code: Select all

IF zcl_company_model=>is_acme_corp(
    iv_bukrs = bukrs ) EQ abap_true...
Better still. Doesn't assume that Acme is a single Company Code. The method encapsulates the requisite logic. Inside could be a constant, multiple constants, Z* table lookup, or a call to a BRFplus rule. The caller doesn't need to know or care.
For clarity I have shown this as a static method, but arguably should be implemented as a singleton instance method.

However I believe there is one basic flaw in all of the above: the code is asking the answer, not the question.

I am suggesting we don't think in terms of "constants" and "magic values", regardless of how they may be expressed (and that includes Z* tables). Instead we should be thinking in terms of "business logic".

So in the above example, the code should not be asking "is this Acme Corp" ? That just happens to be the answering question at this point in time. It could change in the future, and in ways we cannot predict. In the case of a new Company Code certain pieces of custom logic may still apply to it, but not others.

Instead the code should be driven in terms of "business logic". It should be asking the very specific business logic question: "is the custom widget segment relevant to this IDoc" ? This could be coded as:

Code: Select all

IF zcl_business_model=>is_idoc_company_widget_seg_rel(
    iv_bukrs = bukrs ) EQ abap_true.
Inside the method is the constant(s), Z* table lookup, call to BRFplus rule, or even a call to method is_acme_corp().

Think how easy Baz's life would be if everything had been coded this way. All he would have to do is go through his ZCL_BUSINESS_MODEL class making any adjustments - it would all be done by morning tea time. :D

I would be interested to hear other people's opinions on this. Is this the way forward or am I just being a Tuly Idiot ? Image
Grogan

Award cash if useful.

Rich
Posts: 7116
Joined: Thu Oct 31, 2002 4:47 pm
Location: Liverpool
Contact:

Re: Hardcoding

Post by Rich » Mon Oct 28, 2013 3:23 am

That is a huge bundle of good sense Grogan. Unfortunately you score nil points for posting good sense on a Sunday......
Regards

Rich

Image
Abap KC:http://www.richard-harper.me.uk/Kb
SFMDR:http://www.se37.com

VLozano
Posts: 5142
Joined: Mon Sep 13, 2004 8:17 am
Location: Idiocity
Contact:

Re: Hardcoding

Post by VLozano » Mon Oct 28, 2013 4:32 am

For this kind of things we use Z-configuration tables. It prevents us the use of hardcodings and/or constants. And it allows us to use more values in a single check. What about if your client/user needs to add "OR '0200' to your checking? You need to do the needful and change your code again (with constants, hardcodings or anything else). We just need to add a new row to that table and it works fine.
No testing, no bureocrazy, no documentation. Of course, I'm not an external consulting, and I don't care if it means "no extra bill" for that work ;)
Tuly Idiots
Because we know we are part of the problem

Rich
Posts: 7116
Joined: Thu Oct 31, 2002 4:47 pm
Location: Liverpool
Contact:

Re: Hardcoding

Post by Rich » Mon Oct 28, 2013 6:32 am

VLozano wrote:You need to do the needful and .....
Get stomped all over by Bryson.......
Regards

Rich

Image
Abap KC:http://www.richard-harper.me.uk/Kb
SFMDR:http://www.se37.com

Baz
Posts: 4748
Joined: Fri Nov 08, 2002 5:54 am
Location: He's out there! somewhere!!!!
Contact:

Re: Hardcoding

Post by Baz » Tue Oct 29, 2013 4:38 am

and top it all off, we did not inherit any functional or technical documentation for this...

this is going to take a while.... me thinks the 1 March go-live could be under threat!
although as the blueprint is being signed off, i assume all the Z Programs to be used and changed have been captured... bwwwwwahahahahahahahahaaaaaaaaaaaaaaaa
Baz

AsPiRiNg tUlY iDiOt Image

http://www.catb.org/~esr/faqs/smart-questions.html

Image

check out my Podcasts http://dj-baz.podomatic.com

Baz
Posts: 4748
Joined: Fri Nov 08, 2002 5:54 am
Location: He's out there! somewhere!!!!
Contact:

Re: Hardcoding

Post by Baz » Tue Oct 29, 2013 5:38 am

it gets even better.... i have just found a 28,000 line program that starts off:

Code: Select all

*   Structure declaration for Displaying the output data
    BEGIN OF ty_output_1103,
      bukrs TYPE char10,  "Company Code
      butxt TYPE char30,  "Company Code Description
      tolsl TYPE char10,  "Tolerance Key
      wert1 TYPE char25,  "Lower Check limit
      proz1 TYPE char25,  "LowerTolerance limit%
      wert2 TYPE char25,  "Upper Check limit
      proz2 TYPE char25,  "Upper Tolerance limit%
    END OF ty_output_1103,
There is more comments round the tables:

Code: Select all

*    Work area declaration for string table
     W_STR_1401 TYPE TY_STR_1401,

*    Work area declaration for final output table
     W_OUTPUT_1401 TYPE TY_OUTPUT_1401,
*    Work area declaration for Chat of account
     W_T004_1401 TYPE ty_T004_1401,

*    Work area declaration for Chat of account names
     W_T004T_1401 TYPE T004T,

*    Work area declaration for transaxtion key Names
     W_T030W_1401 TYPE T030W,

*    Work area declaration for Rules
     W_T030R_1401 TYPE T030R,

*    Work area declaration Standard Accounts Table
     W_T030_1401 TYPE T030,
then round the actual code! FFS!!!
:evil: :evil: :evil: :evil: :evil: :evil: :evil: :evil:
Baz

AsPiRiNg tUlY iDiOt Image

http://www.catb.org/~esr/faqs/smart-questions.html

Image

check out my Podcasts http://dj-baz.podomatic.com

Post Reply