0

May I use this template method pattern for separate logic from logging and exception handling or it is "bad practice"?

For example I have this code:

public abstract class Parent {
    private final Logger log = LoggerFactory.getLogger(getClass());

public void eat() {
    try {
        doEat();
    } catch (SomeException1 e) {
        log.debug("some text");
        throw new CustomException1();
    }
}

protected abstract void doEat();

public void sleep() {
    try {
        doSleep();
    } catch (SomeException2 e) {
        log.debug("some text");
        throw new CustomException2();
    }
}

protected abstract void doSleep();
}

And my child class:

public class Child extends Parent {
@Override
protected void doEat() {
    //some logic
}

@Override
protected void doSleep() {
    //some logic
}}

I would not have different implementations of methods doEat() and doSleep().

I want to know whether it's worth it and is it 'bad practice' or not.

Sergey Frolov
  • 1,317
  • 1
  • 16
  • 30
  • 2
    Logging is a cross-cutting concern, and as such, is typically handled using [AOP](https://en.wikipedia.org/wiki/Aspect-oriented_programming). – jaco0646 May 28 '16 at 14:55

1 Answers1

1

If the pattern solves the problem you are having, and you are confident it won't cause more problems later, I don't see any issue.

My personal preference here would be for a Decorator pattern. In the "Template" version, where Child extends Parent, the logging behaviour isn't really separate from the logic, it's just hidden. The only benefit is that it's reusable in multiple subtypes of Parent. For them to be truly separate would mean you could change either of them, independently, without the client needing to know. This is how a "Decorator" version might work:

public class Thing implements MyBehaviour {
    @Override
    public void eat() {
        //some logic
    }
}

Then, for the logging:

public class LoggingThing implements MyBehaviour {
    public MyBehaviour delegate;

    public LoggingThing(MyBehaviour delegate) {
        this.delegate = delegate;
    }

    @Override
    public void eat() {
        try {
            delegate.eat();
        } catch (MyException e) {
            // extra behaviour
        }
    }
}

Now the logging behaviour is completely separate from the "eat" behaviour. You can have a MyBehaviour that logs, you can have a MyBehaviour that doesn't log, and you can have a MyBehaviour that does any other thing you want it to do. The client never needs to know which one it has.

Prefer association over inheritance.

Sam
  • 8,330
  • 2
  • 26
  • 51