A dangerous assumption

[Michael_Keller1]

Dear community, the Clean ABAP styleguide recommends to carefully check if a Singleton design pattern should be applied. Some time ago I've seen a use case of Singleton with a dangerous assumption that finally led to an error. I've set up a similar example for discussion. Here it is...

Our class ZCL_ANY_CLASS uses the Singleton design pattern for some unknown reason 🙂

CLASS zcl_any_class DEFINITION

                    PUBLIC

                    FINAL

                    CREATE PRIVATE.



  PUBLIC SECTION.

    DATA mv_company_code TYPE bukrs READ-ONLY.



    CLASS-METHODS get_instance

      IMPORTING

        iv_company_code  TYPE bukrs

      RETURNING

        VALUE(ro_result) TYPE REF TO zcl_any_class.



    METHODS constructor

      IMPORTING

        iv_company_code TYPE bukrs.



  PRIVATE SECTION.

    CLASS-DATA mo_instance TYPE REF TO zcl_any_class.



ENDCLASS.





CLASS zcl_any_class IMPLEMENTATION.



  METHOD get_instance.

    IF mo_instance IS NOT BOUND.

      mo_instance = NEW zcl_any_class( iv_company_code ).

    ENDIF.



    ro_result = mo_instance.

  ENDMETHOD.



  METHOD constructor.

    mv_company_code = iv_company_code.

  ENDMETHOD.



ENDCLASS.

The class is used as follows. Our "virtual" client in our demo scenario currently only has one active company code. So far all right.

DATA lo_any_class TYPE REF TO zcl_any_class.



lo_any_class = zcl_any_class=>get_instance( '1000' ).

cl_demo_output=>write_data( lo_any_class->mv_company_code ).



cl_demo_output=>display( ).

Here's our demo output.

To complicate things now (as life is like that): Later this class is used by another developer in another application. There are now several active company codes in our "virtual" client. The dangerous assumption: what works for one company code is sure to work for several.

TYPES: BEGIN OF ty_company_code,

         company_code TYPE bukrs,

       END OF ty_company_code.



TYPES ty_company_codes TYPE TABLE OF ty_company_code WITH KEY company_code.



DATA lo_any_class TYPE REF TO zcl_any_class.



DATA(lt_company_codes) = VALUE ty_company_codes( ( company_code = '1000' )

                                                 ( company_code = '2000' ) ).



LOOP AT lt_company_codes INTO DATA(ls_company_code).

  CLEAR lo_any_class.

  cl_demo_output=>write_data( ls_company_code-company_code ).

  lo_any_class = zcl_any_class=>get_instance( ls_company_code-company_code ).

  cl_demo_output=>write_data( lo_any_class->mv_company_code ).

ENDLOOP.



cl_demo_output=>display( ).

Here's our demo output.

Ouch! The first call of GET_INSTANCE binds our instance to company code "1000". The second call for company code "2000" only returns the instance for company code "1000".

A separate method "SET_COMPANY_CODE" could be used here to solve our problem - but that's maybe a poor solution according to the design. Another solution could be the use of the Multiton (see the comment of jorgen_lindqvist41). In short: The original design with the IMPORTING parameter via GET_INSTANCE/CONSTRUCTOR no longer fits.

What is still quite manageable in this example can be difficult to recognize in practice. I guess that in an implementation of the INITIALIZE method of the BAdI "ME_PROCESS_PO_CUST" something like this can also be problematic. As long as purchase orders are only worked for one company code, that's okay. As soon as you open the purchase order from another company code in the same working session, you have a problem because of the Singleton.

 

Best regards, thanks for reading and stay healthy

Michael

 

P. S.: Not tired of reading? Check this blog by christoph.pohl.