Application Development Blog Posts
Learn and share on deeper, cross technology development topics such as integration and connectivity, automation, cloud extensibility, developing at scale, and security.
cancel
Showing results for 
Search instead for 
Did you mean: 
hardyp180
Active Contributor
2,460

Design Patterns in ABAP – Visitor (Part 3)




To recap, the aim of the game is to solve a real business problem I encountered (albeit heavily disguised in my examples) by using the classic “Visitor” pattern. This is to see if this design pattern has any practical use in an ABAP environment.

The problem to be solved is thus – we have several different types of monster making machines, designed by various different manufacturers. They all have their slight quirks on so need different classes to handle those quirks. Furthermore no doubt each manufacturer will introduce more changes, as time goes by, some will and some won’t and naturally the time of the changes will be different between manufacturers. So far so good, that is why we have a different class for each type of machine, each implementing the same public interface so all makes of machine present a common appearance to the outside world (other programs which might make use of them).

The problem comes in that each country where a Monster might be created has different laws and these laws have to be applied no matter the make of machine e.g. no monsters with one eye to be created in Tasmania. These laws will of course change over time at a different rate than the changes being made by the manufacturers.

We could have all the logic in the machine classes, but the SOLID (as a rock) rules of OO dictate that there should only ever be one reason a class should change. In this case our machine class should change when the nature of the machine changes, and if anything else changes then it should be left alone.

Nonetheless somehow we need to tell the machines about the country specific laws. In this design pattern we “visit” each machine and tell it about the laws, and it changes its behaviour accordingly, and then the visitor departs after having been served a rather cheeky red wine and a plate of cheese and crackers. The actual code in the machine class has not been changed at all. We have another class to deal with country specific laws, which only changes for one reason, the reason being the laws of that country change.

Moreover looking in our crystal ball we can see that at some unspecified point in the future, some other unspecified factor will start to influence the behaviour of the monster machine as well. This could be anything, the time of day, the madness level of the scientist who operates the machine, Mars being in conjunction with Pluto, the Rooster crowing three times before dawn and the hens laying three addled eggs. We just do not know. However we want to future proof our design so that when this change does arrive we are ready and do not need to modify the monster making machine classes.

In the first blog I created the unit test framework that lets us know if our design is solving the stated business problem, before even attempting to solve that problem.

https://blogs.sap.com/2017/01/13/design-patterns-in-abap-visitor-part-1-of-3/

In the second blog I translated an example of the Visitor Pattern I found in a book by Robert Martin (Uncle Bob) from Java into ABAP until such time as it made the unit tests pass i.e. the code was clearly one possible way to solve the problem at hand.

https://blogs.sap.com/2017/01/18/design-patterns-in-abap-visitor-part-2-of-3/

All well and good, but I was not entirely happy with the design of the solution. Actually I am never happy with any design, ever, let to my own devices I would never finish off any program but keep messing with it forever. It is lucky such things as deadlines exist, even self-imposed ones. In any event, in this blog I will see if I cannot improve the design until I am bit happier with it, which will most likely taking an axe to the existing design and rebuilding from scratch.

Chicken Tikka, Tell me What’s Wrong

I am enchained by my own sorrow in the current design, in my eyes there is no hope for tomorrow. So let us recap the problems so they can be dealt with:-

  • Each monster machine making class has unique attributes which sometimes need to be adjusted. In the Java language the Visitor achieves this by “overloading” where many methods can have the same name and which one gets called depends on the type of the parameter being passed in. There is no proper equivalent of overloading in ABAP.

  • Even if there was, it would mean the “Machine Visitor” interface changing every time there was a new make of machine, which is the main drawback of the Visitor pattern as implemented in Java.

  • In our class under test when we are deciding which type of machine to use, and which type of visitor to create (based on the country) we have lots of horrible CASE statements. That would need to be adjusted every time we expand into a new country or buy a new type of monster making machine, which is a Bad Thing.


 

As it turns out, neither problem is an insurmountable obstacle. Let us take them one at a time.

 

You are Just Not My Type

 

Whenever there is a new law passed in France relating to the making of monsters by mad scientists (which is all the time of course) then the “French Machine Adapter” class has to change which is fine and just what you would expect. We would not expect any other class at all to have to change as a result, not even in the slightest. You would have to create one or more new unit test methods of course, but that is no bad thing, you should do that whenever the rules change.

 

Since we are just changing one class, and it only has one method, we can change it to our hearts content. In this case France has decreed that whilst making monsters if a monster making machine is in operation and it can play music then it must play music by UK band “Black Lace” from the 1980’s so that the mad scientist and his hunchback can sing along and do the actions to “Agadoo” whilst engaged in their dangerous lunatic experiments.

 

In the same way if a monster making machine can let off fireworks whilst in operation in France those fireworks must now by law be Red, White and Blue to match the Tricolour.

 

For the last twenty odd years there have been 28 unelected bureaucrats sitting in a room in Brussels making “good, modern” laws like this, which various countries have to obey e.g. “all fire extinguishers must be red, to increase the chances of you spraying water on an electrical fire in an emergency”. Amazingly last year the people of the UK decided they didn’t like a constant stream of sensible laws like that and voted to henceforth have their own elected lunatics from the UK create new laws for the UK, rather than unelected lunatics from other countries. The rest of the world is laughing at them (the UK voters, not the lunatics) now, especially here in Australia.

 

Oooo Oooo (stands for Object Orientated) bit of politics, bit of politics, and my name’s Ben Elton, goodnight.

 

Anyway, can we please get back to the point? There is a serious problem to solve here and wittering on about EU laws that mandate how bent a banana can be is not going to solve our problem with the specific attributes in each type of monster making machine class.

 

If we are going to do this, let us do this right, and recode the unit test first, so we know what we are aiming for. Naturally, it will fail, as we have not solved the problem yet. Below is just the changed portion of one of the test class methods:-

 

DATA: acme_machine   TYPE REF TO zcl_acme_machine,
bloggs_machine TYPE REF TO zcl_bloggs_machine.

CASE class_under_test->machine_make.
WHEN 'ACME'.
cl_abap_unit_assert=>assert_equals( act = target_data-max_lightning_bolts
exp = 5
msg = 'Incorrect Maximum Lightning Bolts' ).

acme_machine ?= class_under_test->monster_machine.
WHEN 'BLOGGS'.
cl_abap_unit_assert=>assert_equals( act = target_data-max_lightning_bolts
exp = 10
msg = 'Incorrect Maximum Lightning Bolts' ).
bloggs_machine ?= class_under_test->monster_machine.
ENDCASE.

IF class_under_test->country = 'FR'.
IF acme_machine IS BOUND.
cl_abap_unit_assert=>assert_equals( act = acme_machine->get_disco_album( )
exp = 'Black Lace'
msg = 'Incorrect Disco Album Playing' ).
ELSEIF bloggs_machine IS BOUND.
cl_abap_unit_assert=>assert_equals( act = bloggs_machine->get_firework_colour( )
exp = 'Red,White and Blue'
msg = 'Incorrect Fireworks Exploding' ).
ENDIF."Machine Make
ENDIF."France

ENDMETHOD."Then Target Data is Correct

 

The test fails, as well it might. Firstly in real life you probably want to split that method up into several methods, you should only have one test method testing one sort of ASSERTION. Doing too many in one method makes you assert-ified lunatic.

 

In the test class above we know what exact class we are expecting so the cast is likely to work and if it does not the test will fail, which is just the sort of result we want. At runtime however life is more difficult, you cannot go round making such assertions, as that would lead to assertion death.

 

Luckily for us, a new construct in ABAP 7.50 enables us to do just what we require. This is because we can tell the exact class of a reference variable using the TYPE OF or IS INSTANCE OF keywords. In lower releases you would have to go round the house using dynamic run time identification (RTTI) but it could still be done.

 

In a 7.5 system consider the following rewrite of our “Visit” method in the “French Machine Adapter” class:-

METHOD zif_monster_machine_visitor~visit.

* Adapt shared attributes of class to be visited
DATA(adapted_instructions) = io_machine_to_be_visited->get_instructions( ).

adapted_instructions-mortgage_ability = '1'."Cannot Sell Mortgages

io_machine_to_be_visited->set_instructions( adapted_instructions ).

* Adapt class-specific attributes of class to be visited
DATA: acme_machine   TYPE REF TO zcl_acme_machine,
bloggs_machine TYPE REF TO zcl_bloggs_machine.

CASE TYPE OF io_machine_to_be_visited.
WHEN TYPE zcl_acme_machine.
acme_machine   ?= io_machine_to_be_visited.
acme_machine->set_disco_album( 'Black Lace' ).
WHEN TYPE zcl_bloggs_machine.
bloggs_machine ?= io_machine_to_be_visited.
bloggs_machine->set_firework_colour( 'Red,White and Blue' ).
WHEN OTHERS.
RETURN.
ENDCASE.

ENDMETHOD."Visit (from ZCL_FRENCH_MACHINE_ADAPTER

When the unit tests are run again, everything passes, so we can be sure this new solution works and I did not even have to get my axe out, Eugene.

It could be said this is the ABAP equivalent of overloading, but as mentioned earlier the case statement above violates the “one method does one thing” rule as you have to check what has been passed in and then react differently, so that method does radically different things depending on the input.

If I wanted to be anal about this then the one thing the “Visit” method would do is determine the exact class and then call an appropriate private method to deal with that class. That way all the methods would still do one thing only and the main “Visit” method would just provide passage to the correct method in the French Adapter Class. At the moment though I am not feeling anal enough to code that “passage” version of the method.

The first two problems have been solved – the Visitor can not only handle the specific attributes of individual classes but also you don’t have to change the Machine Visitor interface when a new type of machine comes along. You have to change the adapter classes – if and only if the new machine has something new they want to change – but that is fine and dandy.

Another problem is having horrible CASE statements all over the shop to determine the correct class for the monster machine and the country specific adapter.

The last one is how to deal with the nebulous uncertain future requirements, but let us deal with the CASE in point first.

Industrial Machine

What can we use to call a specific class based on a field value like country? Does this situation sound familiar? What can we use – a banana, a BRFPlus rule, a configuration table, a gigantic CASE statement? Ideally we would like something where we can add an entry for the new country (say) and not have to change any code in the calling classes.

You are always going to need some sort of logic; we just do not want to have that logic repeated all over the shop. The solution to this is the good old factory pattern, whereby we do not do a CREATE OBJECT directly; we just pass in some values and get the correct object type back as a result.

We will deal with the monster making machine itself first, and then see if we cannot do something really groovy with the monster machine visitor class.

The class under test (monster mapper) is a local class, so the monster making machine factory class is going to be a local class as well for this example. In the code below you will wonder why I am bothering, I am just moving the CASE statement up one level. I am taking my CASE to a higher court.

CLASS lcl_machine_factory DEFINITION.
PUBLIC SECTION.
CLASS-METHODS: return_machine_for IMPORTING io_mapper      TYPE REF TO lcl_monster_mapper
RETURNING VALUE(ro_machine) TYPE REF TO zif_monster_machine.
ENDCLASS.

CLASS lcl_machine_factory IMPLEMENTATION.

METHOD return_machine_for.

CASE io_mapper->machine_make.
WHEN 'ACME'.
ro_machine = NEW zcl_acme_machine( ).
WHEN 'BLOGGS'.
ro_machine = NEW zcl_bloggs_machine( ).
ENDCASE.

ENDMETHOD.

ENDCLASS.

Then, in the monster mapper class, instead of the CASE statement we have just abstracted, now there is the following:-

monster_machine = lcl_machine_factory=>return_machine_for( me ).

monster_machine->set_instructions( rs_result ).

In real life I would only use a CASE statement if the logic was trivial. Alternate solutions in increasing order of difficulty are configuration tables, BRFPlus rules and filter BADIs. All of those mean you no longer have to change the code when a new class comes along, but you have to trade off the extra effort vs how often you would add a new class. Filter BADIs in particular seem a sledgehammer to crack a nut in this case. On the other hand I am in love with abstraction; but it won’t even return my phone calls.

So in this example I just move the CASE statement. After having made the change, I run the unit tests again, nothing breaks, all well and good.

Having a factory method, I could now enforce the singleton pattern if I so desired, only ever having one instance of the monster machine, if that was important e.g. representing the real world more accurately, as if there is only one monster machine in reality, why would you have two instances at runtime potentially with different states?

Why I am passing the whole class in, as opposed to just the make? That way, if later on the logic to return the correct type of machine becomes more complicated, then the call to the factory does not have to be changed, as the factory has access to all the public information in the interface class. That makes this factory specific to the monster mapper class, which is the downside. I am sure there is a happy medium, I just have not thought of it yet.

Honey Nut Loops

I still have one CASE statement left to determine the concrete class of the monster machine visitor. I intend to do the same trick with a factory class, but now I start to think about my final requirement which is to future proof this against new requirements.

We only have one type of “adapter” visitor class at the moment, and since the monster can only be made in one country at a time we only pass back one visitor. However if we have a new requirement e.g. changing the behaviour of the mapper based on the lunacy level of the mad scientist, then we could quite possibly pass back more than one instance of different classes that implements the Visitor interface. We could of course pass back none at all, if the country (or lunacy level) does not require any changes to the mapping.

So I think instead of a single class instance, the factory return a table of instances of classes which implement the machine visitor interface.

“Just hold on one second!” I hear you scream in anger “What are you going to do? Pass in the Mapping Class again? That would make the visitor class not re-usable in the slightest”.

Indeed it would, and that would be a Bad Thing. We want to pass in only the “filter” parameters just like we do with a BADI but without all the rigmarole. What do we know that takes in a table of random parameters? It is the good old application log. Let us pass in a table typed after BAL_T_PAR with what we are filtering by and the filter value.

TYPES: l_tt_visitors TYPE STANDARD TABLE OF REF TO zif_monster_machine_visitor WITH DEFAULT KEY.

CLASS lcl_machine_visitor_factory DEFINITION.
PUBLIC SECTION.
CLASS-METHODS: return_visitors_for IMPORTING it_filters         TYPE bal_t_par
RETURNING VALUE(rt_visitors) TYPE l_tt_visitors,
add_country_visitor IMPORTING id_country  TYPE land1
CHANGING  ct_visitors TYPE l_tt_visitors,
add_loony_visitor   IMPORTING id_sanity   TYPE zde_sanity_description
CHANGING  ct_visitors TYPE l_tt_visitors.

ENDCLASS.

When I come to code the implementation of the methods (the skeletons being created for me by ABAP in Eclipse) I find that I can use a whole bunch of the new syntax in ABAP 7.40 and above to keep the lines of code to a bare minimum.

CLASS lcl_machine_visitor_factory IMPLEMENTATION.

METHOD return_visitors_for.

LOOP AT it_filters INTO DATA(filters).
CASE filters-parname.
WHEN 'COUNTRY'.
add_country_visitor(
EXPORTING id_country  = CONV land1( filters-parvalue )
CHANGING  ct_visitors = rt_visitors ).
WHEN 'LUNACY'.
add_loony_visitor(
EXPORTING id_sanity   = CONV zde_sanity_description( filters-parvalue )
CHANGING  ct_visitors = rt_visitors ).
WHEN OTHERS.
CONTINUE.
ENDCASE.
ENDLOOP.

ENDMETHOD.

METHOD add_country_visitor.

CASE id_country.
WHEN 'FR'.
APPEND NEW zcl_french_machine_adapter( ) TO ct_visitors.
WHEN 'US'.
APPEND NEW zcl_usa_machine_adapter( )    TO ct_visitors.
ENDCASE.

ENDMETHOD.

METHOD add_loony_visitor.
* Placeholder for future logic0
ENDMETHOD.

ENDCLASS."Machine Visitor Factory

If any of the statements above seem strange to you, as the syntax is not familiar just leave a comment at the end of the blog and I will attempt to explain.

The point is that if the country where the monster is being made does not need any adjustments no visitor will be called – just like a filter BADI. The difference is if you pass in more than one filter criteria you might get all sorts of different visitor classes back.

I run the unit tests again – all is well, nothing is rotten in the State of Denmark.

With this design you do not need to change the calling code when a new country is added – just the visitor factory class (unless you have a configuration table or some such). However doubtless you will have spotted I am not yet passing in the looniness value. Well done! You get a feather, and a set of diamond encrusted steak knives made of solid gold, and an all expenses paid holiday to Mars to meet the Ice Warriors.

So if the day dawns when the looniness becomes important I will have to change the mapping class such that it keeps track of the inventors madness level (no way around that), and then amend the code that fills the table that calls the visitor class which was swallowed by the old lady to catch the dog to catch the cat to catch the spider to catch the fly, I don’t know why she swallowed a fly, perhaps she’ll die.

However maybe I was keeping track of the lunacy level in the mapping class for some other purpose? If I was it would seem a crying shame to have to change the code when we already know the value.

Can you hear me calling? Can you hear me calling you?

In Graham Robinson’s Gateway design pattern, which I am using at work, he has a method called CALL_ALL_GETTERS which dynamically loops through all the elements of a structure and calls any GET method which may exist to retrieve the value of the attribute for a given class instance.

In this case I do not have the attributes in a structure – they are all individual attributes. I could (if we were talking about a Z class) do some sort of fancy dancy thing to dynamically read all the attributes of my class by reading table SEOCOMPO (who was in “Last of the Summer Wine”) and pass them into the filter table, or just pass them all in anyway by listing them all in the code, and the off-chance one day they may be important. The latter is probably the easiest.

However we do not go to the Moon because it is easy. We do this because it is difficult. So go on, go on, fill up the filter table dynamically, and leave me breathless. I dare you. I double dare you.

In the interim, here is my final “DO MAPPING” method, with the country filter value hard coded, and no looniness passed in as that attribute is not in the mapping class as yet.

METHOD do_mapping.

MOVE-CORRESPONDING is_source_data TO rs_result.

monster_machine = lcl_machine_factory=>return_machine_for( me ).

monster_machine->set_instructions( rs_result ).

DATA(filter_values) = VALUE bal_t_par(
( parname = 'COUNTRY' parvalue = country ) ).

DATA(visitors) = lcl_machine_visitor_factory=>return_visitors_for( filter_values ).

LOOP AT visitors INTO DATA(machine_visitor).
machine_visitor->visit( monster_machine ).
ENDLOOP.

rs_result = monster_machine->get_instructions( ).

ENDMETHOD.

For the moment I am happy bunny with my implementation of the Visitor pattern. I think I have stayed due to the original intention of the pattern, used it on what is an actual real world business problem, and overcome all the problems I thought about earlier in this blog.

The last remaining topic is to question whether we actually need to use these design patterns in ABAP in the first place.

Are you being Visited? Are you free Mr. ABAP? I’m Free!

The visitor pattern, just like its mate the Decorator pattern, exists to fulfil the “open closed” principle, and the idea of making a program do something new without changing it.

Initially you might think that is a contradiction in terms but if you think for just a little bit, SAP is full of such things.

The most obvious is the IMG enabling multiple industries and countries to run the same transaction e.g. VA01 and having it react very differently based on configuration. Function module based user exits and BADIs are the same sort of thing. I don’t count the enhancement framework in this basket because with an implicit enhancement you are in fact changing the program, as you are with form based user exits like MV45AFZB.

The Visitor pattern as described in the literature I have seen involves having a “Visitor” interface specific to the class being visited. That class in turn has an “accept” method and that lets its behaviour be modified by any future class that desires to do so by that class implementing the specific visitor interface to “visit” the already existing unchangeable class and stuff around with all its private data to its hearts content.

That would seem, on the face of it, to break the “encapsulation” principle which SAP restricts by giving the BADIS defined interfaces saying what you can and cannot change, It is more like the good old MV45AFZB type of exit where you can change anything at all in the VA01 program and make the data utterly inconsistent if you get things wrong. In my example above I have the monster machine class control what can be changed by any visitors by wrapping SET and GET methods around the attributes which can be modified.

This does beg the question though, if my monster machine class is controlling what can and cannot be changed, do I need a Visitor pattern at all? Can’t I just put BADIs in my custom code so some code later on in a new BADI implementation can change the values? Or is the concept of the BADI the manifestation of the Visitor pattern in ABAP, in the same way that having ABAP classes be able to raise events enables the Observer pattern?

So I would like to end this series of blogs by asking all and sundry what they think on this matter? Is the Visitor pattern applicable to ABAP? If so, does my implementation make any sense, or have I got totally the wrong end of the stick? And in case there is some value in what I have done, have I made any fundamental errors or can you see any areas for improvement?

Cheersy Cheers

Paul
2 Comments
Labels in this area