V_B V_B - 1 month ago 6
Java Question

Single responsibility principle and right way to organize interfaces for DI

Today morning I was shocked by the dullnest of the question I gonna ask you. If the answer is obvious - pls comment and I'll delete the question ASAP.

Suppose you have files which are stored on disk and you need to load them. Files are of 3 types with extensions

*.ext1
,
*.ext2
and
*.ext3
. So I have three
load
methods, each of them should be synchronized by different lock. I should inject somehow this behavior into my service with DI framework. Which of the ways below are better?

It seems the first way is an obviuos. But what makes me uncertain - principle of single responsibility and the fact you'll never want to load all three extensions: you will load only 1 & 3 or only 2 & 3.

1) General interface for all file types loader:

public interface FileLoader {
void loadFilesOfExt1();
void loadFilesOfExt2();
void loadFilesOfExt3();
}

//The most obvoius solution, but doesn't it break the principle of single responsibility?
//It also has one more dwawback - I want to synchronize each method by different lock, and it requires using of locks
public class FileLoaderImpl implements FileLoader {
public void loadFilesOfExt1() { /* impl */ }
public void loadFilesOfExt2() { /* impl */ }
public void loadFilesOfExt3() { /* impl */
}


2) Specific Interface for each type of files:

//It looks pretty weird to have three interfaces with the same API
public interface Ext1Loader { void load(); }
public interface Ext2Loader { void load(); }
public interface Ext3Loader { void load(); }
public class Ext1LoaderImpl extends Ext1Loader { public synchronized void load() { /* impl */ } }
public class Ext2LoaderImpl extends Ext2Loader { public synchronized void load() { /* impl */ } }
public class Ext3LoaderImpl extends Ext3Loader { public synchronized void load() { /* impl */ } }


3) Hierarchy of interfaces:

//Isn't it an overengeneering? 3 Empty interfaces
public interface FileLoader { void load(); }
public interface Ext1Loader extends FileLoader {}
public interface Ext2Loader extends FileLoader {}
public interface Ext3Loader extends FileLoader {}
public class Ext1LoaderImpl extends Ext1Loader { public synchronized void load() { /* impl */ } }
public class Ext2LoaderImpl extends Ext2Loader { public synchronized void load() { /* impl */ } }
public class Ext3LoaderImpl extends Ext3Loader { public synchronized void load() { /* impl */ } }


4) Single interface:

//This makes me feel like all implementations `Ext1Loader`, `Ext2Loader` and `Ext3Loader` are interchangable.
//But actually you may want to inject `Ext1Loader` nad `Ext3Loader` simultaneously into your class
public interface FileLoader { public void load(); }
public class Ext1Loader implements FileLoader { public synchronized void load() { /* impl */ } }
public class Ext2Loader implements FileLoader { public synchronized void load() { /* impl */ } }
public class Ext3Loader implements FileLoader { public synchronized void load() { /* impl */ } }

Answer

The third solution looks like the sanest solution here. But adding marker interfaces which do nothing seems to be an overengineering.

I would suggest you consider my viewpoint of your task:

public interface FileLoader {
    void load();
}

There is only one interface, implementations which will present loading a file of a specific format. Each loader has a unique name that describes which file type the loader will work with.

@Component("Ext1")
public class Ext1FileLoader implements FileLoader {
    public synchronized void load() {}
}

After that, we need a factory that produces loaders on demand.

@Component
public class FileLoaderFactory implements ApplicationContextAware {

    // to get new loaders
    private ConfigurableApplicationContext context;

    // to keep already pulled out instances
    private Map<String, FileLoader> loaders;

    public FileLoader getFileLoaderByFileType(String type) {
        // trying to use a lazy-loading mode

        return loaders.get(type) == null ? getFromContext(type) : loaders.get(type);
    }

    private FileLoader getFromContext(String type) {
        // getting a bean from the context, putting it into a map and returning a value

        FileLoader loader = context.getBean(type, FileLoader.class);
        loaders.put(type, loader);
        return loader;
    }

    @Override
    public void setApplicationContext(ApplicationContext context) throws BeansException {
        // a casting here is to get broader functionality from a context instance

        this.context = (ConfigurableApplicationContext) context;
    }

}

This solution could have been more concise, but we wouldn't have been able to turn a lazy-init mode on. Take a look:

@Component
class FileLoaderFactory {

    // it would collect all loaders available in the content
    private @Autowired Map<String, FileLoader> loaders;

    public FileLoader getFileLoaderByFileType(String type) {
        return loaders.get(type);
    }

}