CODEWITHSUNDEEP
javamethodsrefactoringcoding-style
Genzer
Genzer

Reputation: 2991

Can we refactor these methods?

I have classes with methods implemented as follow:

void methodOne() {
    try {
        getHelper().doActionOne();
    } catch ( Exception ex ) {
        throw new CustomException( ex );
    }
}

void methodTwo() {
    try {
        getHelper().doActionTwo();
    } catch ( Exception ex ) {
        throw new CustomException( ex );
    }
}

void methodThree() {
    try {
        getHelper().doActionThree();
    } catch ( Exception ex ) {
        throw new CustomException( ex );
    }
}


void methodFour;
void methodFive;
...

Is there a better way to do this? These codes make me uncomfortable.

EDIT: Sorry for unclear example. I'm implementing GenericDao class with Hibernate, the real code is something like this:

class GenericDaoImpl<T, PK> {

    PK create( T object ) {
        try {
           getSession().save( object );
        } catch( Exception ex ) {
           throw new DataAccessLayerException( ex );// wrap any exception to my exception
        }
    }

   T read( PK id ) {
       try {
           getSession().get( T.class, id );
       } catch ( Exception ex ) {
           throw new DataAccessLayerException( ex );
       }

   }

  void update( T object );
  void delete( T object );

}

Upvotes: 3

Views: 276

Answers (3)

David Harkness
David Harkness

Reputation: 36562

This facility is provided by the Spring Framework along with many others. First, it has a specific HibernateTemplate which maps each Hibernate-specific exception to an unchecked, related Spring exception. Second, it provides an AOP service to translate exceptions at the method level so you can specify the mappings once and apply them uniformly across multiple services.

While I wouldn't use Spring for just this one feature, it has huge benefits for building applications and I have continued using it for many years.

Upvotes: 0

Edwin Buck
Edwin Buck

Reputation: 70959

Yes, you can always refactor code. The only issue is whether you're going to refactor to make it better or worse. The hints that this code is odd, is a good hint that it can be made much better.

This looks like a good candidate for polymorphisim. Instead of five different methods, try five different classes with one shared method. The interface will tie it all together.

public interface DoIt {
  public void doIt();
}

public class One implements DoIt {
  public void doIt() {
    // code which was previously in getHelper.doActionOne();
  }
}

public class Two implements DoIt {
  public void doIt() {
    // code which was previously in getHelper.doActionTwo();
  }
}

...

public class Five implements DoIt {
  public void doIt() {
    // code which was previously in getHelper.doActionFive();
  }
}

Now the only thing is to create the right class for the situation, and call its doIt() method.

Upvotes: 1

Andy White
Andy White

Reputation: 88475

Just a basic suggestion, but you could refactor this into something like a "Command Pattern." This pattern allows you to encapsulate some functionality into a class that implements a single method. The class can be instantiated and passed into another class to be executed, and the executor class doesn't have to know or care what it's doing, it just needs to call execute(). If the actions require arguments, the classes that implement Command can include fields/properties that can be set in the constructor or by standard property setters.

Make an interface like this (my Java is rusty, so this may not be 100% valid syntax):

public interface Command
{
    public void execute();
}

public class ActionOne implements Command
{
    public void execute()
    {
        // do actionOne...
    }
}

public class ActionTwo implements Command
{
    public void execute()
    {
        // do actionTwo...
    }
}

// etc. for more actions

Then create the class that executes the action, and the calling code just needs to pass in the correct Command implementation class.

public class Executor
{

    public void executeCommand(Command command)
    {
        try
        {
            // Put any boilerplate code here (logging, auditing, etc.)
            command.execute();
        }
        catch (Exception ex)
        {
            // Put general error handling code here.  If you're just catching and rethrowing, consider not catching the Exception at this level.  If it's a checked exception, add a throws clause to the method.
            throw new CustomException();
        }
    }
}

Upvotes: 6

Related Questions