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: 

How to design reusable exception classes

fabianlupa
Contributor

Using the object oriented exception concept I sometimes find myself in a situation that looks like the following.

I define a method that uses an exception class in its signature to indicate some kind of error. So for example this:

CLASS lcl_partner_finder DEFINITION FINAL CREATE PRIVATE.
  PUBLIC SECTION.
    CLASS-METHODS:
      "! Find a business partner id for a debtor
      "! @parameter iv_debtor | Debtor id
      "! @parameter rv_partner | Found business partner id
      "! @raising lcx_bp_not_found | No business partner exists for iv_debtor
      get_partner_from_debtor IMPORTING iv_debtor         TYPE kunnr
                              RETURNING VALUE(rv_partner) TYPE bu_partner
                              RAISING   lcx_bp_not_found.
ENDCLASS.

According to the guidelines for exceptions I have to pick which of the 3 base classes my new exception class LCX_BP_NOT_FOUND uses. In this particular case I think that it would be reasonable to pick CX_STATIC_CHECK because the user of this method cannot validate beforehand if the partner exists, at least not using this class (so no reason for CX_DYNAMIC_CHECK). One could even argue that this method could be used for that exact reason. CX_NO_CHECK is out of the question in my opinion.

So I create the exception class:

"! Business partner not found exception
CLASS lcx_bp_not_found DEFINITION
  FINAL
  CREATE PUBLIC
  INHERITING FROM cx_static_check.

  PUBLIC SECTION.
    METHODS:
      constructor IMPORTING ix_previous       LIKE previous OPTIONAL
                            iv_identification TYPE csequence OPTIONAL,
      get_text REDEFINITION.
    DATA:
      mv_identification TYPE string READ-ONLY.
ENDCLASS.

Because of the static check any time I call get_partner_from_debtor without a try-catch that includes lcx_bp_not_found or propagation of the exception I will get a syntax warning. Which is what I want, because the probability of it happening is quite high, so the caller should be forced to handle this exception.

Later on in my implementation I define a new method to do something else.

"! Check if a business partner has a specified role (currently active)
"! @parameter iv_partner | Partner id
"! @parameter iv_role | Role to check
"! @parameter rv_role_present | Role is present
partner_has_role IMPORTING iv_partner             TYPE bu_partner
                           iv_role                TYPE bu_role
                 RETURNING VALUE(rv_role_present) TYPE abap_bool.

Now the first step in implementing it would be to validate the import parameters. So I would check if the partner specified in iv_partner exists. And if it does not I would not continue in the method and instead I would like to raise an exception indicating this error. Above I have already defined an exception class for the case that a business partner could not be found. But in this second method the perspective has changed. I would argue that because iv_partner is a direct importing parameter it is the responsibility of the caller to retrieve the bp id. Most of the users of this method would probably be annoyed if they got a warning to catch a LCX_BP_NOT_FOUND exception because they have implemented code before calling it that guarantees a valid iv_partner actual parameter. According to the guidelines this is the exact use case of CX_DYNAMIC_CHECK exceptions. But my exception is already inheriting from CX_STATIC_CHECK.

My question is, what is the recommended action in this situation? I have the same exception, with the same attributes, the same additional methods, maybe the same T100 message class behind it, but looked at from two different angles.

Approaches I have thought of so far:

  1. Copy the exception class and change the base class, i. e. LCX_BP_NOT_FOUND_DYN_CHECK => Code duplication, increased maintenance, redundancy, not an option
  2. Live with many TRY. ... CATCH lcx_bp_not_found ##NO_HANDLER. ENDTRY. blocks obfuscating the code base
  3. Raise a generic dynamic check exception instead, i. e. LCX_ILLEGAL_ARGUMENT and reference the static check one using the PREVIOUS import parameter and pass through the texts with an additional parameter and lx_bp_not_found->get_text( ) (losing the message class reference and longtext information when displaying the exception via MESSAGE)

I most of the time opt for the third approach. One does run into the risk of accidentally propagating the exception though, because the calling method might use LCX_ILLEGAL_ARGUMENT as well on its own. So I am not certain as to what is the ideal solution for this problem.

I am interested in any comments or recommendations.

4 REPLIES 4

michał_badura
Participant

Very good question indeed. But why don't You define an interface with the common texts, which then could be implemented by all Your exception classes, irrespectively from which of the three basis classes they inherit. After all, a text is nothing more but a constant of a special type.

constants:
  begin of common_text,
    msgid type symsgid value 'ID',
    msgno type symsgno value '001',
    attr1 type scx_attrname value '',
    attr2 type scx_attrname value '',
    attr3 type scx_attrname value '',
    attr4 type scx_attrname value '',
  end of common_text.

Hmm, that might solve the T100 texts problem (you would also have to define the attributes for the message variables in the interface too), but extra methods of the exception class would have to be duplicated.

After thinking about it, the best solution I can come up with right now is to adjust my api design a bit. For example not finding a business partner in get_partner_from_debtor might also return an initial value instead of raising the exception. Or if CVI and MDS are set up properly not finding a partner for a debtor might be such an exceptional case that CX_DYNAMIC_CHECK is the correct parent class.

I agree with returning an initial value instead of raising the exception. If you know how to handle a special situation/event correctly in the current context, then there is no need to propagate an exception to the caller.

What might work is a variant of the Null Object pattern: make sure all your code can handle the Null partner (a partner with initial value) as a valid partner, e.g. partner_has_role( ) will just return abap_false for the Null partner.

So your logic will behave as expected without exceptions.

JNN

RieSe
Contributor
0 Kudos

Good overall approach 🙂