Application Development Discussions
Join the discussions or start your own on all things application development, including tools and APIs, programming models, and keeping your skills sharp.
cancel
Showing results for 
Search instead for 
Did you mean: 

Issue with changing attribute of a singleton

nikolayevstigneev
Contributor
0 Kudos

Hi folks!

I ran into a weird behaviour of a class implementing a singleton pattern.

So, what have we got:

there is a class, say,  zcl_my_fi_log. It's main usage is collecting messages during the documents processing. I believe that a singleton is a pretty nice candidate for that role - whatever place there is an error (or whatever) message, it collects them all.

Here's how it looks like (after removing unnecessary stuff):

CLASS zcl_my_fi_log DEFINITION
   FINAL
   CREATE PRIVATE.

   PUBLIC SECTION.

     DATA mt_log TYPE bapirettab.

     CLASS-METHODS factory
       RETURNING
         VALUE(ro_obj) TYPE REF TO zcl_my_fi_log .

     METHODS add_sy_message .

     METHODS display .

     CLASS-DATA mo_log TYPE REF TO zcl_my_fi_log .
ENDCLASS.

CLASS zcl_my_fi_log IMPLEMENTATION.

   METHOD factory.

     IF NOT mo_log IS BOUND.
       CREATE OBJECT mo_log.
     ENDIF.

     ro_obj = mo_log.

   ENDMETHOD.

   METHOD add_sy_message.

     DATA ls_return TYPE bapiret2.

     CALL FUNCTION 'BALW_BAPIRETURN_GET2'
       EXPORTING
         type   = sy-msgty
         cl     = sy-msgid
         number = sy-msgno
         par1   = sy-msgv1
         par2   = sy-msgv2
         par3   = sy-msgv3
         par4   = sy-msgv4
       IMPORTING
         return = ls_return.

       APPEND ls_return TO mt_log.

   ENDMETHOD.

   METHOD display.

     CALL FUNCTION 'SUSR_DISPLAY_LOG'
       EXPORTING
         display_in_popup = abap_true
       TABLES
         it_log_bapiret2  = mt_log.

   ENDMETHOD.

ENDCLASS.

It behaves as a singleton is supposed to, e.g. if I code something like this

   DATA lo_my_fi_log1 TYPE REF TO zcl_my_fi_log.
   DATA lo_my_fi_log2 TYPE REF TO zcl_my_fi_log.

   MESSAGE s000(zfi) WITH 'lalala'.
   zcl_my_fi_log=>factory( )->add_sy_message( ).

   lo_my_fi_log1 = zcl_my_fi_log=>factory( ).

   lo_my_fi_log1->mt_log[ 1 ]-type = 'E'.

   lo_my_fi_log2 = zcl_my_fi_log=>factory( ).

   lo_my_fi_log2->mt_log[ 1 ]-type = 'W'.

   zcl_my_fi_log=>factory( )->display( ).

we'll see that message type of the instance held by zcl_my_fi_log is changed (as far as the message type of local classes). Of course, direct changing of class attributes is a bad practice, but I prefer less code to demonstrate the problem, let's live without getter/setter methods in this example


As far as I have a method returning the instance of the class how about removing my local variables like this


MESSAGE s000(zfi) WITH 'lalala'.
zcl_my_fi_log=>factory( )->add_sy_message( ).
 
LOOP AT zcl_my_fi_log=>factory( )->mt_log ASSIGNING FIELD-SYMBOL(<ls_log>)
   WHERE type = 'S'.

   <ls_log>-type = 'E'.
ENDLOOP.

LOOP AT zcl_my_fi_log=>factory( )->mt_log ASSIGNING <ls_log>
   WHERE type = 'S'.
   " Shouldn't be here! But check it out in debugger...
   <ls_log>-type = 'E'.
ENDLOOP.

zcl_my_fi_log=>factory( )->display( ).


And finally we see message type 'S' as if we haven't changed it earlier.


Guys, do you have any ideas of what happens? Isn't my class a singleton? What is the problem eliminated by explicit instantiation?


P.S. I'm playing with ABAP 7.40 SP9.

1 ACCEPTED SOLUTION

SuhaSaha
Advisor
Advisor
0 Kudos

Hello Nikolay,

Finally something to tingle the greycells

I think i have an answer, let me try to break it down for you -

    1. You are using the functional method call in the LOOP construct: zcl_my_fi_log=>factory( )->mt_log
    2. You are trying to modify the contents of the int. table mt_log in the LOOP.

Now let us see what ABAP documentation has to say about it -

  1. Call of a functional method in a suitable reader position, where the return value of the method is used as an operand. ABAP Keyword Documentation
  2. Reader position: Operand position in which the operand is read-only. ABAP Keyword Documentation

The int. table mt_log is never modified during the loop execution. You should have checked it in the debugger.

Workaround: Declare the singleton (instance) of the class as public/read-only and use it directly. (Similar to the AGENT attribute of the agent classes for persistence classes)


LOOP AT zcl_my_fi_log=>mo_log->mt_log ASSIGNING <ls_log> WHERE type = 'S'.

    <ls_log>-type = 'E'.

  ENDLOOP.

BR,

Suhas

PS - I see it is an ABAP trapdoor, maybe you should blog about it

Message was edited by: Suhas Saha

7 REPLIES 7

SuhaSaha
Advisor
Advisor
0 Kudos

Hello Nikolay,

Finally something to tingle the greycells

I think i have an answer, let me try to break it down for you -

    1. You are using the functional method call in the LOOP construct: zcl_my_fi_log=>factory( )->mt_log
    2. You are trying to modify the contents of the int. table mt_log in the LOOP.

Now let us see what ABAP documentation has to say about it -

  1. Call of a functional method in a suitable reader position, where the return value of the method is used as an operand. ABAP Keyword Documentation
  2. Reader position: Operand position in which the operand is read-only. ABAP Keyword Documentation

The int. table mt_log is never modified during the loop execution. You should have checked it in the debugger.

Workaround: Declare the singleton (instance) of the class as public/read-only and use it directly. (Similar to the AGENT attribute of the agent classes for persistence classes)


LOOP AT zcl_my_fi_log=>mo_log->mt_log ASSIGNING <ls_log> WHERE type = 'S'.

    <ls_log>-type = 'E'.

  ENDLOOP.

BR,

Suhas

PS - I see it is an ABAP trapdoor, maybe you should blog about it

Message was edited by: Suhas Saha

0 Kudos

Workaround: Declare the singleton (instance) of the class as public/read-only and use it directly.

... and instantiate it in the class_constructor

0 Kudos

But if the CONSTRUCTOR of your singleton has a parameter interface you cannot use the CLASS_CONSTRUCTOR. A separate GET_INSTANCE method is needed in that case.

But yeah, in this example the OP could have instantiated it in the static constructor.

0 Kudos

Holy moly!

Thanks for clarifying the situation! I'll check the doc more carefully.

I thought of blogging the question but finally decided that the first thing to do is to understand what's going on

Speaking about the workaround - I believe that the mentioned example is quite "dirty" and one shouldn't modify class attributes in such a way.

But if you still want to do so for your unclear purposes (like one of my mates who asked me about this) - this seems to be a pretty nice place for shooting yourself in the foot

Former Member
0 Kudos

Hi Nikolay,

I think ABAP acts pretty weired here. I adapted your code a little:

MESSAGE s000(zfi) WITH 'lalala'

zcl_my_fi_log=>factory( )->add_sy_message( ).

.

LOOP AT zcl_my_fi_log=>factory( )->mt_log ASSIGNING FIELD-SYMBOL(<ls_log>).

     <ls_log>-type = 'E'.

ENDLOOP.

data(lo_obj) = zcl_my_fi_log=>factory( ).

LOOP AT lo_obj->mt_log ASSIGNING <ls_log>

      WHERE type = 'S'.

     " Shouldn't be here! But check it out in debugger...

     <ls_log>-type = 'E'.

ENDLOOP.



The first loop does not change the singletons state. The second loop does change the state, namely the internal table mt_log.

It interesting to see that the field symbol <ls_log> is undefined after loop one, but is still defined after loop two.


I think in the first loop abap sees the functional method call ( to factory( ) ) and actually creates a temporary copy of mt_log (?!).


Pretty strange behaviour to me.


Maybe we should ask Mr. ABAP a.k.k Horst Keller?







0 Kudos
It interesting to see that the field symbol <ls_log> is undefined after loop one, but is still defined after loop two.

Thanks for mentioning it! I forgot to do so in my reply

0 Kudos

Well I didn't see your answer when I started to create my answer ...