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.

I am creating a skeleton application to demonstrate use of the Strategy design pattern and inversion of control inside a Strategy.

The application is the backend of a simple board game, which also has the capability to save and load the board. Since we can have different types of data stores for saving, I have used an interface PersistenceStrategy and concrete implementations of it like the FilePersistenceStrategy.

I have listed the code below. However, I am stuck in one place. Inside the FilePersistenceStrategy class, the save() as well as load() methods need to communicate with a file. I do not want these methods to create a file handle (Reader/Writer) themselves, since this will be bad for testability.

I know there are several ways for these methods to get hold of FileReader and FileWriter objects - like getting them from a factory, or getting the FQN from a properties file and instantiating them using reflection.

The most common method of solving this problem - Dependency Injection, cannot be used here because the load() and save() methods come from an interface and we cannot add Strategy specific parameters to the interface.

How would you solve this problem, and what in your opinion are the pros and cons of different approaches ?

Code follows:

Board.java

package com.diycomputerscience.example.sanddi;

public class Board {

    private PersistenceStrategy persistenceStrategy;
    private BoardState state;

    public Board(PersistenceStrategy persistenceStrategy) {
        this.persistenceStrategy = persistenceStrategy;
    }

    //various methods of playing the board game

    public void load() throws PersistenceException {
        this.state = this.persistenceStrategy.load();
    }

    public void save() throws PersistenceException {
        this.persistenceStrategy.save(this.state);
    }
}

BoardState.java

package com.diycomputerscience.example.sanddi;

public class BoardState {

}

PersistenceStrategy

package com.diycomputerscience.example.sanddi;

public interface PersistenceStrategy {
    public void save(BoardState state) throws PersistenceException;
    public BoardState load()  throws PersistenceException;
}

FilePersistenceStrategy.java

package com.diycomputerscience.example.sanddi;

import java.io.FileReader;
import java.io.FileWriter;
import java.io.IOException;
import java.io.Writer;

public class FilePersistenceStrategy implements PersistenceStrategy {

    @Override
    public void save(BoardState state) throws PersistenceException {
        FileWriter writer = getFileWriter();
        //logic to save the state 
        if(writer != null) {
            try {
                writer.close();
            } catch(IOException ioe) {
                String msg = "Could not close FileWriter";
                throw new PersistenceException(msg, ioe);
            }
        }
    }

    @Override
    public BoardState load() throws PersistenceException {
        FileReader reader = getFileReader();
        BoardState state = parseFileForBoardState(reader);
        return state;
    }

    private BoardState parseFileForBoardState(FileReader reader) {
        // TODO Auto-generated method stub
        return null;
    }

    /**
     * HOW DO I USE INVERSION OF CONTROL HERE FOR BETTER TESTABILITY ?
     * @return
     */
    private FileReader getFileReader() {
        // Create a FileReader from a well known file name... 
        // perhaps a hardcoded name or read name from a config file
        return null;
    }

    /**
     * HOW DO I USE INVERSION OF CONTROL HERE FOR BETTER TESTABILITY ?
     * @return
     */
    private FileWriter getFileWriter() {
        // Create a FileWriter from a well known file name... 
        // perhaps a hardcoded name or read name from a config file
        return null;
    }
}

PersistenceException.java

package com.diycomputerscience.example.sanddi;

public class PersistenceException extends Exception {
    //Override all constructors from the superclass

    public PersistenceException(String msg, Throwable cause) {
        super(msg, cause);
    }
}
share|improve this question
    
Thanks for your question! I'm not sure whether this is on-topic or not. Code should already be working according to the faq. –  Quentin Pradet May 31 '12 at 11:21
    
@Cygal I see what you mean, but the design is working. The methods which return null, do so only for brevity. I can change them to return actual readers and writers if required. –  Parag May 31 '12 at 11:29
1  
I wasn't sure, which is why I didn't vote for a close. Thanks for the clarifications. –  Quentin Pradet May 31 '12 at 13:27
    
I suppose if you really needed the flexibility, you could inject some sort of FileLocator / FileNamer / WriterFactory or similar strategy. But do you really need that kind of flexibility? You're going to have to stop and test the writes at some point, and I would have expected it to be at the FilePersistenceStrategy level (that is, you're mocking PersistenceStrategy objects for testing the rest of your code, right?). –  Clockwork-Muse May 31 '12 at 16:13
    
How is the strategy created? –  dreza Nov 12 '12 at 22:19
add comment

2 Answers

up vote 2 down vote accepted

For me the question is how the strategy is created. Is it possible to inject the file name during construction of the strategy? That would be the way I would do it.

Else you can create callbacks that are created and injected. When you need the file call this callback and let this decide (eg asking the user).

share|improve this answer
    
I think I agree. Something somewhere must know about the different strategies. Depends on if this something could be altered accordingly –  dreza Nov 12 '12 at 22:18
add comment

Inject the file information into the constructor. You are going to end up with something (a factory, an IoC container, etc) that knows which strategy to use; it can also know the file information.

I like to wrap file access in a mockable class too, but that might be a problem that is specific to C#.

share|improve this answer
add comment

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.