Propagating static check exceptions or raising unchecked exception in low-level private methods

[SuhaSaha]

As per this comment in this clean ABAP discussion thread, the ideal scenario to handle exceptions would be like this:

class LCX_APPLICATION_ERROR definition inheriting from CX_STATIC_CHECK.
endclass.

class LCX_INTERNAL_ERROR definition inheriting from CX_NO_CHECK.
endclass.

class LCL_START definition create public.
  public section.
    methods TOP_METHOD raising LCX_APPLICATION_ERROR.
  private section.
    methods MIDDLE_METHOD.
    methods LOW_METHOD raising LCX_INTERNAL_ERROR.
endclass.

class LCL_START implementation.
  method LOW_METHOD.
    " Raise unchecked error
    raise exception type LCX_INTERNAL_ERROR.
  endmethod.

  method MIDDLE_METHOD.
    " Since LOW_METHOD raises an unchecked exception we do not need to handle or propagate it
    LOW_METHOD( ).
  endmethod.

  method TOP_METHOD.
    try.
        MIDDLE_METHOD( ).
      catch LCX_INTERNAL_ERROR into data(L_INTERNAL_ERROR).
        " Unwrap the internal error and link it to the application error
        raise exception new LCX_APPLICATION_ERROR( PREVIOUS = L_INTERNAL_ERROR ).
    endtry.
  endmethod.
endclass.

Let's say that LOW_METHOD calls a procedure which raises a static exception CX_COLLABORATOR_ERROR.

What would be in your opinion a better/cleaner way of propagating it to the TOP_METHOD?

1. Catch the exception & wrap it inside the unchecked exception?

  method LOW_METHOD.
    try.
        new COLLABORATOR( )->DO_SOMETHING( ).
      catch CX_COLLABORATOR_ERROR into data(COLLABORATOR_ERROR).
        raise exception new LCX_INTERNAL_ERROR( PREVIOUS = COLLABORATOR_ERROR ).
    endtry.
  endmethod.

2. Catch the exception & wrap it inside a static exception?

Of course, this would mean adding the LCX_APPLICATION_ERROR exception to the signature of the LOW_METHOD & MIDDLE_METHOD.

  method LOW_METHOD.
    try.
        new COLLABORATOR( )->DO_SOMETHING( ).
      catch CX_COLLABORATOR_ERROR into data(COLLABORATOR_ERROR).
        raise exception new LCX_APPLICATION_ERROR( PREVIOUS = COLLABORATOR_ERROR ).
    endtry.
  endmethod.

BR,

Suhas

PS: IMO, propagating the exception AS-IS is not a clean solution. Therefore i have not considered about it.

[matt]

It's a bit of an artificial scenario (naturally). Normally, you'd expect there to be some contextual information at each raise. As such, I'd always want to pass up the internal error information. So option 1 for me.

[SuhaSaha]

Thanks for the response Matt. For the sake of clarity i did not add too much noise (contextual info) in the raise.

Assuming that there is some context to the raise exception, why would you prefer raising an unchecked exception over a static exception? The advantage i see is that the middle method(s) need not declare the unchecked exception explicitly in their signature.

BR,

Suhas

[matt]

Oh, I see. Well, I always use static exceptions so the question doesn't arise for me. 😉

But conceptually, I don't think the internal methods should know about lcx_application_error. It feels wrong.

[SuhaSaha]

I have been using static exceptions primarily. I use dynamic exceptions for error situations which could have been prevented by the caller.

The idea of unchecked exceptions came to my mind after reading this discussion on the clean ABAP Github page.

[r_lindemann]

I don't think it matters whether the exception is thrown by LOW_METHOD itself, or by a procedure called by LOW_METHOD - either way it's LOW_METHOD's to handle (or, in this case, propagate). Now, whether you best do this via a checked or an unchecked wrapper exception, IMHO depends on two things: whether it's a functional or a runtime exception (see below), and whether LOW_METHOD is private or public.

I tend to think of exceptions in two categories:

• functional or application-logic exceptions, denoting (possibly frequent) situations that the caller may be responsible for and/or able to handle (like a "not found", "not valid" or the likes); these should always be declared on the interface
• runtime environment or technical exceptions, where there's really not much a caller can do; (rare) DB, file system, communication, time-out problems fall into this category. I can see why one might use unchecked exceptions here.

My opinion on this is not yet completely formed, but I feel that when a method is intended to be used by "other" code (and public methods generally are), and therefore has some sort of API character, then all exceptions a caller has to be able to deal with are part of the contract and should therefore be declared in the procedure's interface. If on the other hand a method is private, you can pretty much do as you like.

(Edit: just realized that LOW_METHOD is actually private - my bad. Therefore rewrote the below.)

So, for your question, considering that LOW_METHOD is private, you could very well get away with an unchecked wrapper exception. As long as you make sure that it is caught inside the class, and not propagated through the public TOP_METHOD (because of the API considerations above); this may not be easy to guarantee over time, especially if the class is somewhat complex or evolves a lot over time.
So, unless you have a myriad of such low- and mid-level methods that all produce runtime-style exceptions, where you'd save a lot of typing and clutter by using unchecked exceptions, I would still go with checked, because it makes it easier to ensure that no undeclared exceptions are thrown by the class' public methods.

[matt]

Hi Rainer.

[r_lindemann]

Hi Matt! 😄

[SuhaSaha]

Hi Rainer,

Thanks for the feedback. This is a "known" fact that unhandled unchecked exceptions are implicitly propagated.

I think that a simple unit test should prevent such unexpected exceptions leaking out of the top (public) methods.