styleguides icon indicating copy to clipboard operation
styleguides copied to clipboard

Use sub-classes to enable callers to distinguish error situations

Open d-schaaf opened this issue 3 years ago • 4 comments

Hi all,

in the first coding the signature of method generate() is missing. Which exceptions are declared? Only cx_generation_error or also the 2 sublclasses (which is what I would prefer)? What do you think?

d-schaaf avatar Apr 01 '21 10:04 d-schaaf

Yes, let's add that signature https://github.com/SAP/styleguides/pull/192.

Not sure about whether to declare the super-class or list the individual sub-classes as exceptions. My impression is that there are good arguments for both:

By giving the super-class, I declare that the method might raise all different kinds of generation errors. The consumer can react to the ones he wants to handle specifically, but should also be prepared for other cases.

By listing the sub-classes, I declare that the method will only raise these different situations, although there may be many more generation errors that will never occur here.

Both sound reasonable, but have different intentions.

HrFlorianHoffmann avatar Apr 09 '21 09:04 HrFlorianHoffmann

My 2 cents: When only the superclass is listed, can the consumer really react to the ones he wants to handle spoecifically? This would force the consumer to have a look inside the code and see if there are more specific exceptions raised than given in the signature of the method. It's a really hard decision here...

d-schaaf avatar Apr 09 '21 10:04 d-schaaf

A combined option is to list both the specific ones the method uses and the general one of the component. There's a syntax check if I recall correctly forcing you to list the subclasses first but that should be it (or maybe that was for the catch blocks). If you don't declare the specific ones then you also cannot add documentation for them in the signature.

fabianlupa avatar Apr 09 '21 11:04 fabianlupa

@flaiker you are correct, the check is for catch blocks. It warns that the subclass block will never be reached if the superclass is caught first. It's an interesting argument and I don't think there is a catch-all (pardon the pun) answer. A third argument to @HrFlorianHoffmann's list is that the raising method itself may call other methods and thus not be aware of the exception subclasses. I also often address this scenario from the caller's side and look for a common superclass just to 'clean up' my catch block into a single handler if I'm not bothered about details. e.g. I'll use a single CATCH CX_SALV_ERROR around the whole SALV instead of catching all the individual SALV exceptions in the different parts. F4 is an underrated feature of ADT that really helps here.

pokrakam avatar Apr 10 '21 05:04 pokrakam