current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 1 down vote favorite

I call several WebServices which return each a different exception type:

try {
   //calling first WS1 here
} catch (my.package1.FaultWS_Exception e) {
        StringBuilder sb = getMessageFromException(e);
        log(sb);
}

try {
   //calling first WS2 here
} catch (my.package2.FaultWS_Exception e) {
        StringBuilder sb = getMessageFromException(e);
        log(sb);
}

my getMessageFromException simply returns a pretty printed message from the exception.

But as my exceptions are in different packages (each method is in a different WS), I had to create different getMessageFromException() functions for each exception:

private StringBuilder getMessageFromException(my.package1.FaultWS_Exception e) {
    StringBuilder sb = new StringBuilder();
    if (e.getFaultInfo() != null) { //returns my.package1.FaultWS
        sb.append(e.getFaultInfo().getDescfault());
        sb.append("\n");
        sb.append(e.getFaultInfo().getCodefault());
        sb.append("\n");
    } else if (e.getMessage() != null) {
        sb.append(e.getMessage());
        sb.append("\n");
    }
    return sb;
}

private StringBuilder getMessageFromException(my.package2.FaultWS_Exception e) {
    StringBuilder sb = new StringBuilder();
    if (e.getFaultInfo() != null) { //returns my.package2.FaultWS
        sb.append(e.getFaultInfo().getDescfault());
        sb.append("\n");
        sb.append(e.getFaultInfo().getCodefault());
        sb.append("\n");
    } else if (e.getMessage() != null) { 
        sb.append(e.getMessage());
        sb.append("\n");
    }
    return sb;
}

This code looks very ugly and a bit repetitive (I have 4 different signatures so far), is there a way to refactor it ? knowing that the Exceptions are defined the same, except from the package name.

share|improve this question

migrated from stackoverflow.com May 9 at 14:46

This question came from our site for professional and enthusiast programmers.
add comment

2 Answers

up vote 2 down vote accepted

If the faultInfo properties are identical for every case, you could use reflection:

try {
   //calling first WS1 here
} catch (my.package1.FaultWS_Exception e) {
        StringBuilder sb = getMessageFromException(e, e.getFault());
        log(sb);
}

try {
   //calling first WS2 here
} catch (my.package2.FaultWS_Exception e) {
        StringBuilder sb = getMessageFromException(e, e.getFault());
        log(sb);
}

...

private StringBuilder getMessageFromException( Exception e, Object faultInfo )
{
    StringBuilder sb = new StringBuilder();
    if ( faultInfo != null )
    {
        FaultInfo genericFault = asGenericFault(faultInfo);
        sb.append( genericFault .getDescfault() );
        sb.append( "\n" );
        sb.append( genericFault .getCodefault() );
        sb.append( "\n" );
    }
    else if ( e.getMessage() != null )
    {
        sb.append( e.getMessage() );
        sb.append( "\n" );
    }
    return sb;
}

private FaultInfo asGenericFault( Object fault )
{
    try
    {   
        Class<? extends Object> faultType = fault.getClass();
        String descfault = (String) faultType.getMethod( "getDescfault").invoke( fault );
        String codefault = (String) faultType.getMethod( "getCodefault").invoke( fault );

        return new FaultInfo(descfault, codefault);
    }
    catch ( Exception e )
    {
        throw new IllegalArgumentException( "expected a type with public getDescfault() and getCodefault() methods but got: " + fault, e );
    }
}

FaultInfo would be a simple Java Bean with the 2 properties you need.

share|improve this answer
    
yep that's a very good way to do that. Thanks –  Majid L May 9 at 23:02
add comment
up vote 3 down vote

Simply use one method for all type of Exceptions.

Make it public static and move into a utility class.

public static StringBuilder getMessageFromException(Exception e){...}

Why do you need overloaded methods for different type of Exceptions?

User defined Exceptions are also subtype of Exception itself.

--EDIT--

create an interface for all your custom exceptions having getFaultInfo() method.

interface MyFaultWS {} // marker interface
class FaultWS implements MyFaultWS {} // do it for both the packages

interface FaultInfoException {FaultWS getFaultInfo();}

// do it for both the packages
class FaultWS_Exception extends Exception implements FaultInfoException {

    @Override
    public FaultWS getFaultInfo() {
        return ...; // return your package specific exception
    }
}

Finally use 

public static StringBuilder getMessageFromException(FaultInfoException e){...}
share|improve this answer
    
Thanks for the answer. I wouldn't be able to call e.getFaultInfo() as it doesn't exist in Exception. And also Exception is not really a tag relevant to this question :) –  Majid L May 6 at 22:07
    
Have a look at my updated post. –  Braj May 6 at 22:24
    
well the FaultWS are automatically generated so I cannot modify it. But I will accept the answer if there's no better one. –  Majid L May 6 at 22:55
    
Finally you can do one thing extract the part that is common in all the methods and put in a separate method to reduce the redundant code. –  Braj May 6 at 22:56
    
what's the part that is common in that particular case? –  Majid L May 6 at 23:27
show 2 more comments

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.