How do I model shared code pieces differing only slightly at a specific point effectively?

Viewed 44

I am writing a data export application that is used for many scenarios (Vendors, Customers, Cost Centers, REFX Contracts, etc). In the end there are two main ways of exporting: save to file or call webservice.

So my idea was create an interface if_export, implement a class for each scenario. The problem is, the call webservice code differs slightly at the point of the actual call: the called method has a different name each time.

My ideas for dealing with this so far are:

  1. Abstract cl_webservice_export with subclasses for each scenario. Overwrite method containing the actual call.
  2. cl_webservice_export with member type if_webservice_call. class for each scenario implementing if_webservice_call method call_webservice()
  3. Dynamic CALL METHOD webservice_instance->(method_name) inside concrete cl_webservice_export method containing the actual call and passing (method_name) to cl_webservice_export.

My code: export_via_webservice is the public interface provided by cl_webservice_export or via if_export

      METHODS export_via_webservice
      IMPORTING
        VALUE(it_xml_strings)    TYPE tt_xml_string_table
        io_service_consumer      TYPE REF TO ztnco_service_vmsoap
      RETURNING
        VALUE(rt_export_results) TYPE tt_xml_string_table.


        METHOD export_via_webservice.
    
        LOOP AT it_xml_strings INTO DATA(lv_xml_string).
          call_webservice(
            EXPORTING
              io_service    = io_service_consumer
              iv_xml_string = lv_xml_string-xmlstring
            RECEIVING
              rv_result     = DATA(lv_result)
          ).
          rt_export_results = VALUE #( BASE rt_export_results (
                                                lifnr = lv_xml_string-xmlstring
                                                xmlstring = lv_result ) ).
        ENDLOOP.
    
      ENDMETHOD.

Actual webservice call, overridden or provided by if_webservice_call

    METHODS call_webservice
      IMPORTING
        io_service       TYPE REF TO ztnco_service_vmsoap
        iv_xml_string    TYPE string
      RETURNING
        VALUE(rv_result) TYPE string.

    METHOD call_webservice.
    TRY.
        io_service->import_creditor(
          EXPORTING
            input              = VALUE #( xml_creditor_data = iv_xml_string )
          IMPORTING
            output             = DATA(lv_output)
        ).
      CATCH cx_ai_system_fault INTO DATA(lx_exception).
    ENDTRY.

    rv_result = lv_output-import_creditor_result.

    ENDMETHOD.

How would you solve this problem, maybe there are other, better ways of doing it?

1 Answers

I know three common patterns to solve this question. They are, in ascending order of quality:

Individual implementations

Create one interface if_export, and one class that implements it for each web service export variant that you need, i.e. cl_webservice_export_variant_a, cl_webservice_export_variant_b, etc.

enter image description here

Major advantages are the intuitive simplistic class design and complete independence of the implementations that avoids accidental spillover from one variant to the other.

Major disadvantage are the probably massive portion of code duplication between the different variants, if their code varies in only few, minor positions.

You already sketched this as your option 2, and also already highlighted that it is the least optimal solution for your scenario. Code duplication is never welcome. The more so since your web service calls vary only slightly, in some method name.

In summary, this pattern is rather poor, and you shouldn't actively choose it. It usually comes into existence on its own, when people start with variant a, and months later add a variant b by copy-pasting the existing class, and then forgetting to refactor the code to get rid of the duplicate parts.

Strategy pattern

This design is commonly known as the strategy design pattern. Create one interface if_export, and one abstract class cl_abstract_webservice_export that implements the interface and includes most of the web service-calling code.

Except for this detail: The name of the method that should be called is not hard-coded but retrieved via a call to a protected sub-method get_service_name. The abstract class does not implement this method. Instead, you create sub-classes of the abstract class, i.e. cl_concrete_webservice_export_variant_a, cl_concrete_webservice_export_variant_b, etc. These classes implement only the inherited protected method get_service_name, providing their concrete needs.

enter image description here

Major advantages are that this pattern completely avoids code duplication, is open for further extensions, and has been employed successfully in lots of framework implementations.

Major disadvantage is that the pattern starts to erode when the first variant arrives that does not completely fit, e.g. because it does not only vary the method name, but also some parameters. Evolving then requires in-depth redesign of all involved classes, which can amount to considerable cost. Another disadvantage is that the inheritance setup can make it cumbersome to write unit tests: for example, unit-testing the abstract class requires to make up a test double that sub-classes it and overwrites the protected method with sensing and mocking code - all possible but not as neatly as with interfaces between the classes.

You already sketched this as your option 1. In summary, I would recommend to choose this pattern if you have control over all involved classes and are willing to spend some extra-effort to keep the pattern clean in case it doesn't fit completely.

Composition

Composition means avoiding inheritance in favor of loose interaction between indepdent classes over classes. Create the interface if_export and individual concrete implementations of it as cl_webservice_export_variant_a, cl_webservice_export_variant_b, etc.

Move out the shared code to a class cl_export_webservice_caller that receives whatever data and variant (e.g. method name) it needs. Let the variant classes call this shared code. To complete the class design, introduce another interface if_export_webservice_caller that decouples the variants classes from the caller class.

enter image description here

The major advantages are that all classes are independent from each other and can be recombined in several different ways. For example, if in the future you need to introduce a variant X that would call its web service in a completely different way, you can simply add it, without having to redesign any of the other involved classes. In contrast to the strategy pattern, writing unit tests for all involved classes is trivial.

There are no real disadvantages to this pattern. (The seeming disadvantage that it needs one more interface is not really one - object orientation has the aim to clearly separate concerns, not to minimize the overall number of classes/interfaces, and we shouldn't be afraid to add more of those if it adds clarity to the overall design.)

This option sounds similar to the option 3 you sketched, but I am not 100% sure. Anyway, this would be the pattern I would vote for.

Related