2

Suppose I need to combine 2 apis which throw Exceptions on basically every method. The code usually looks cluttered like this:

class MultiApi{

   Api1 api1;
   Api2 api2;

   //I led the caller handle problems
   Api2Data doSomethingOrThrow() throws Api1Ex, Api2Ex {
       byte[] data = api1.getData();
       return api2.handleData(data);
   }

   //I try to recover
   Api2Data somethingElse(){

       try{ 
         byte[] data = api1.getData();
         return api2.handleData(data);
       catch (Api1Ex e){ // handle 
       } catch (Api2Ex e) //handle
       }
       // if not handled
       return null;
   }
}

So I thought I could write an ExceptionWrapper like this:

class Wrapper extends Exception{

    private Exception mTrigger;

    public Wrapper(Api1Ex e) {
        mTrigger = e;
    }

    public Wrapper(Api2Ex e) {
        mTrigger = e;
    }

    public int getCode(){
        if (mTrigger instanceof Api1Ex) {
           return ((Api1Ex)mTrigger).getCode();
        } else {
            return ((Api2Ex)mTrigger).getCode();
        }
    }
}

and use it like this:

   Api2Data doSomethingOrThrow() throws Wrapper {
       try{

           byte[] data = api1.getData();
           return api2.handleData(data);
       catch (Api1Ex | Api2ex e){
           throw new Wrapper(e);
       ]
   }

But Java is complaining that it cannot resolve the type. I'm also not able to use the neat multi-catch syntax in my Constructor:

Wrapper(Api1Ex | Api2Ex e){
    mTrigger = e;
}

So I need to Write something like this:

Wrapper(Exception e){
    if (e instanceof Api1Ex || e instanceof Api2Ex) {
        mTrigger = e;
    } else {
       throw new RuntimeException("Got unkown Ex type!");
    }
}

Which is very ugly from my point of view. Is there a better (meaning more elegant) solution to this problem? I usually am not interested, which Api Fails, Only that one of them does, and which errorCode it threw (which have the same meaning in both cases)

I kinda think of a duck-typing feature: Any Exception with a getCode would be sufficient.

EDIT: as many suggest the easiest way would be to implement a common type. But I am not able to modify Api1 or Api2 in any way or form. Also the getCode doesn't return an int, but an enum holding one. The enum types are (of course) not the same but their int representation is.

Rafael T
  • 15,401
  • 15
  • 83
  • 144
  • 1
    Your `Wrapper` exception seems to just provide the `getCode()` method. Why don't you provide a common interface what declares that method and that's implemented by both api exceptions? Taking that a step further: if the exceptions basically differ on their code, why not just provide a generic `ApiException` that takes a `code` parameter? Do you need to handle those exceptions differently and just use the code in the multi-api implementation? – Thomas Aug 30 '19 at 15:02
  • 2
    As for the _multi-catch_ syntax in the _constructor_: that's exactly the point - a multi-catch is for a _catch_-expression and not a constructor. – Thomas Aug 30 '19 at 15:04
  • If the operation that you want to perform in the case of each exception doesn't change, you could use some form of method interceptor to implement a fault barrier specifically to catch those two exceptions and then handle them in a generic way. – JonK Aug 30 '19 at 15:07

2 Answers2

2

If you want to employ the multiple-catch block, you need a constructor that takes the least upper bound (LUB) type of Api1Ex and Api1Ex.

The declared type of an exception parameter that denotes its type as a union with alternatives D1 | D2 | ... | Dn is lub(D1, D2, ..., Dn).

https://docs.oracle.com/javase/specs/jls/se12/html/jls-14.html#jls-14.20

There is no way around it. The constructor Wrapper(Api1Ex | Api2Ex e) is never valid.

Overloaded constructors will work if you declare a couple of catch blocks.

try {
    ...
} catch(Api1Ex exception) {
    throw new Wrapper(exception);
} catch (Api2Ex exception) {
    throw new Wrapper(exception);
} 

At some point, you will need to draw a clear line between the part that uses both exceptions and the part that only deals with your wrapper exception. That line may look ugly (like the snippet above), but, at least, it will help you reduce "ugliness" all over the place.

Andrew Tobilko
  • 48,120
  • 14
  • 91
  • 142
  • Im afraid, that that may be the answer I need to go with. Like other half-baked `features` (think of multi-bound generics like `` ) java supplies. One cannot use them in a way the feat makes you think – Rafael T Aug 30 '19 at 15:58
  • @RafaelT don't be afraid of simple (and, sometimes, too straightforward) solutions ;) – Andrew Tobilko Aug 30 '19 at 16:18
1

Of course that is not a valid constructor :

Wrapper(Api1Ex | Api2Ex e){
    mTrigger = e;
}

In your last sample, the creation of an exception that may trigger the throw of another exception if the type of the original exception doesn't match is not the correct way either (while it compiles).
The best way to bound the parameter type of the constructor is using a common base type for the both exceptions and specify it as parameter such as :

private final GenericApiException genericApiException;
WrapperException(GenericApiException genericApiException){
    super(genericApiException);
    this.genericApiException = genericApiException;
}

You can so use that constructor :

catch (Api1Ex | Api2ex e){
  throw new WrapperException(e);
]

WrapperException looks clearer than Wrapper.


the big but is that I cannot modify api1 or 2 in any way or form

In this case, overload the constructor :

WrapperException(Api1Ex e){
    mTrigger = e;
}

WrapperException(Api2Ex e){
    mTrigger = e;
}
davidxxx
  • 125,838
  • 23
  • 214
  • 215
  • sure. I know naming in my sample isn't good. it was just for demo purposes. the big `but` is that I cannot modify api1 or 2 in any way or form – Rafael T Aug 30 '19 at 15:15
  • Yes. This is what I did. I overloaded them, which makes my `catch (Api1Ex | Api2Ex e)` unusable. I have to catch them seperatly just to be able to re-throw it the same way. Its not a problem I couldn't solve. it just clutters the code in a way I feel should be unesessary – Rafael T Aug 30 '19 at 15:20
  • Of course.You should catch each exception individually. But it makes sense because without possibility to refactor the two exceptions, that is the single way to invoke the constructor with the specific type as argument. – davidxxx Aug 30 '19 at 15:26
  • 1
    To not clutter that, you should wrap any invocation that may throw the exceptions to a common method that perform the invocation, return the result and does also the two `catch`s . Something like that as signature : `private T execute (Supplier function) throws WrapperException {...// execute and generic catch and retrow}`. But that is beyond of the original question. – davidxxx Aug 30 '19 at 15:30