Single responsibility policy problem

Question

I'm a bit stuck using the single responsibility policy.

I have a class named Parser, which can be configured to parse input a certain way. For the sake of convenience I will limit the configuration properties to a single property config in the following example:

class Parser {

    boolean config;
    State state;

    AST parse(String input) { ... }
}

I want to reuse the parse instance for multiple parses. The problem is that the state is redundant for the instance itself, however it is required while parsing. Beside the overhead of the state in memory, it doesn't work in multi-threaded environment.

The same goes for a class named Compiler:

class Compiler {

    boolean config;
    State state;

    Code compile(String input) { ... }
}

I was wondering how this is generally solved.

Note: This is maybe a minimal example, but I'm often stuck thinking of a better design.

Answer 1

It is OK if your parser and compiler have state, but there is no need to reuse the objects. What I tend to do is offer a static public method Compiler.compile() or Parser.parse() that takes care of instantiating a stateful object, feeds the input to it, retrieves the result, and returns the result while discarding the object. Since static methods are problematic, we can split the stateful parser thing from the public API that provides the parse() method:

public interface Parser {
  public Ast parse(String input);
}

/*internal*/ interface ParserImpl {
  Ast result();
  void doFoo();
  void doBar();
}

class ParserAPI implements Parser {
  @Override
  public Ast parse(String input) {
    ParserImpl p = makeParserImpl(input):
    p.doFoo();
    p.doBar();
    return p.result();
  }

  protected /*virtual*/ ParserImpl makeParserImpl(String input) {
    return new DefaultParserImpl(input, otherDependency);
  }
}

/*internal*/ class DefaultParserImpl implements ParserImpl {
  State state;

  DefaultParserImpl(String input, Dependency otherDependency) {
    ... // initialize state
  }

  @Override
  Ast result() { ... }

  @Override
  void doFoo() { ... }

  @Override
  void doBar() { ... }
}

What is the point of this indirection? Testability. The ParserAPI class uses the Template Method Pattern so that subclasses can swap out the dependency on the DefaultParserImpl, e.g. to use a mock object. While a user of the Parser can keep calling parse again and again on the same object, in fact we are re-creating a new state for each parse, and don't keep the parse state around after parsing is completed. This not only makes the parser and code that depends on the parser easy to test; creating a fresh object for each parse also makes it easier to reason about correctness.

The disadvantage is that we now have one more class in our system, so we do lose a bit simplicity. However, this is offset by the simpler API that is exposed to a parser user.

Answer 2

I once faced a similar problem. While my solution certainly is not optimal, it did the trick.

I used an additional layer between the actual compiling and the concurrently used Compiler-class. This class would be called Compilation in your example.

/*
 This class exists once for multiple (simultaneous) inputs
*/
public class Compiler{
    CompilationParameters compilationParameters;

    public Compiler(CompilationParameters params){
        ...
    }

    public CompilationResult compile(CompilationInput input){           // Of course you can simplify this to a string

        Compilation compilation = new Compilation(compilationParameters, input);
        compilation.compile();
        return compilation.getResult();
    }
}

/*
 This class exists exactly once per input
*/
public class Compilation{
    CompilationState state;

    public Compiler(CompilationParameters params, CompilationInput input){
        ...
    }
    public void compile(){
        ...
    }
    public CompilationResult getResult(){
        ...
    }
}

Now you could argue, that this violates the Open-Closed-Principle. And you'd be right. This solution can be extended for different types of compilations using a factory-pattern for the creation of the Compilation-object. Imagine the following:

public abstract class Compilation{...};
public class CompiletimeOptimizedCompilation extends Compilation{...};
public class RuntimeOptimizedCompilation extends Compilation{...};
public class CompilationFactory{
    public static Compilation createCompilation(CompilationType type, CompilationParameters params, CompilationInput input)
    {
        switch(CompilationType)
        {
        case eCompileTimeOptimized:
            return new CompileTimeOptimizedCompilation(params, input);
        case eRuntimeOptimized:
            return new RuntimeOptimizedCompilation(params, input);
        case ...
        }
    }
}

In this case Compilation might also be an interface instead of an abstract class. To decide that I'd probably need more details about the implementations. And of course the Compiler-class would boil down to:

public class Compiler{
    CompilationParameters compilationParameters;

    public Compiler(CompilationType type, CompilationParameters params){ // This type CAN be part of the parameters
        ...
    }

    public CompilationResult compile(CompilationInput input){           // Of course you can simplify this to a string

        Compilation compilation = CompilationFactory.createCompilation(type, compilationParameters, input);
        compilation.compile();
        return compilation.getResult();
    }
}

Of course the same works for your Parser-class, but I thought it would be a bad example because the noun of "to parse" is "the parse".

If you have any further questions or remarks, don't hesitate asking.

PS: Please excuse any spelling mistakes. I'm not a native.

Answer 3

If your State is needed only by parse() method, why do you keep it in the instance? Why not to create State objects only when they are really needed? You could then pass them around to the methods, called by parse() if needed.

If most methods inside Parser use State, then you should just not reuse the instance. Object construction is usually very cheap if you keep your constructors lightweight. If configuration of the Parser is complex, use factories to construct them. Use Flyweights if configuration process itself is time-consuming.

If you can avoid using single object from different threads - please do so at all costs. The less synchronization problems you have the better, believe me.