How many times have you seen code like this:
- Code: Select all
FORM user_command_0200.
CASE ok_code.
WHEN 'TSCM'.
PERFORM set_desired_start_screen USING gc_screen_transports
CHANGING gv_start_screen.
PERFORM set_screen_number USING gc_screen_transports
CHANGING dv_dynnr
maintabstrip-activetab.
WHEN 'TSFF'.
PERFORM set_desired_start_screen USING gc_screen_piece_lists
CHANGING gv_start_screen.
PERFORM set_screen_number USING gc_screen_piece_lists
CHANGING dv_dynnr
maintabstrip-activetab.
WHEN 'TSMM'.
PERFORM set_desired_start_screen USING gc_screen_client_requests
CHANGING gv_start_screen.
PERFORM set_screen_number USING gc_screen_client_requests
CHANGING dv_dynnr
maintabstrip-activetab.
WHEN 'TSPD'.
PERFORM set_desired_start_screen USING gc_screen_delivery
CHANGING gv_start_screen.
PERFORM set_screen_number USING gc_screen_delivery
CHANGING dv_dynnr
maintabstrip-activetab.
WHEN 'TSSN'.
PERFORM set_desired_start_screen USING gc_screen_single_request
CHANGING gv_start_screen.
PERFORM set_screen_number USING gc_screen_single_request
CHANGING dv_dynnr
maintabstrip-activetab.
WHEN 'TOGGLE'.
CALL TRANSACTION 'SE09'.
LEAVE PROGRAM.
WHEN OTHERS.
The question I would ask is what do the 4 character OK codes mean ? This also applies to other code as well including function module calls, procedure calls and so on and so forth.
Would it not be easier to understand if it was coded something like this:
- Code: Select all
Module User_Command_0100 input.
Case sy-Ucomm.
When c_Rollback. Perform Check_Back_0100.
When c_Save. Perform Save_Data_0100 Changing w_Ok.
If w_Ok = True.
Leave to screen 0.
EndIf.
When c_Insert_Row. Perform Insert_Line_0100.
When c_Delete_Row. Perform Delete_Line_0100.
When c_TableTop. Move 0 to g_Start.
When c_Up_1_Page. Subtract g_Max_Step_Loop from g_Start.
If g_Start < 0.
Move 0 to g_Start.
EndIf.
When c_Down_1_Page.
Each of the Ok codes has been placed in a constant that gives a good idea of what the code is meant to do:
- Code: Select all
Constants: c_Type_Menu type Seu_Type Value 'MENU', "For Node
c_Type_Command type Seu_Type Value 'CMND', "Types in the
c_Type_Trans type Seu_Type Value 'TRAN', "RS Tree,
c_Type_Title type Seu_Type Value 'TITL',
*
* Colors for different parts of tree display
*
c_Menu_Color type i Value 4,
c_Cmnd_Color type i Value 5,
c_Trans_Color type i Value 6,
c_Title_Color type i Value 3,
*
* Tree list function codes
*
c_Add_Node type syUcomm Value 'ZRAD',
c_Delete_Node type syUcomm Value 'ZRDL',
c_Rename_Node type syUcomm Value 'ZRRN',
c_Move_Node type syUcomm Value 'ZRMV',
c_Mark_Node type syUcomm Value 'TRMK',
c_Expand type syUcomm Value 'TREP',
c_Collapse type syUcomm Value 'TRCM',
c_Pos_Cursor type syUcomm Value 'TRTO',
As you can see, I tend to place a large number of values into constants, and then use that constant where ever I need that value.
This also decreases the maintenance effort required for a program as should a value need to be changed, it only needs to be changed in one place - in the constant definition.
Also, a suggestion from Rob D, which also highlights maintainability:
Rob D wrote:If in a select statement I have to retrieve all the entries of a transparent table where a certain field contains initial values, I use a constant.
- Code: Select all
tables: T5NDK.
CONSTANTS: c_init_bunum LIKE t5ndk-bunum VALUE IS INITIAL.
DATA : i_t5ndk LIKE STANDARD TABLE OF t5ndk,
wa_t5ndk LIKE LINE OF i_t5ndk.
select * from t5ndk into table i_t5ndk where bunum eq c_init_bunum.
Again, it's making the code easier to read and initialises the value correctly without typing loads of zeroes.