What's the best way of partitioning a class when its functionality needs to be externally accessed in different ways by different classes? Hopefully the following example will make the question clear :)

I have a Java class which accesses a single location in a directory allowing external classes to perform read/write operations to it. Read operations return usage stats on the directory (e.g. available disk space, number of writes, etc.); write operations, obviously, allow external classes to write data to the disk. These methods always work on the same location, and receive their configuration (e.g. which directory to use, min disk space, etc.) from an external source (passed to the constructor).

This class looks something like this:

public class DiskHandler {
    public DiskHandler(String dir, int minSpace) {
        ...
    }
    public void writeToDisk(String contents, String filename) {
        int space = getAvailableSpace();
        ...
    }
    public void getAvailableSpace() {
        ...
    }
}

There's quite a bit more going on, but this will do to suffice.

This class needs to be accessed differently by two external classes. One class needs access to the read operations; the other needs access to both read and write operations.

public class DiskWriter {
    DiskHandler diskHandler;

    public DiskWriter() {
        diskHandler = new DiskHandler(...);
    }
    public void doSomething() {
        diskHandler.writeToDisk(...);
    }
}

public class DiskReader {
    DiskHandler diskHandler;

    public DiskReader() {
        diskHandler = new DiskHandler(...);
    }
    public void doSomething() {
       int space = diskHandler.getAvailableSpace(...);
    }    
}

At this point, both classes share the same class, but the class which should only read has access to the write methods.

Solution 1

I could break this class into two. One class would handle read operations, and the other would handle writes:

// NEW "UTILITY" CLASSES
public class WriterUtil {
    private ReaderUtil diskReader;

    public WriterUtil(String dir, int minSpace) {
        ...
        diskReader = new ReaderUtil(dir, minSpace);
    }
    public void writeToDisk(String contents, String filename) {
        int = diskReader.getAvailableSpace();
        ...
    }
}
public class ReaderUtil {
    public ReaderUtil(String dir, int minSpace) {
        ...
    }
    public void getA开发者_JS百科vailableSpace() {
        ...
    }
}

// MODIFIED EXTERNALLY-ACCESSING CLASSES
public class DiskWriter {
    WriterUtil diskWriter;

    public DiskWriter() {
        diskWriter = new WriterUtil(...);
    }
    public void doSomething() {
        diskWriter.writeToDisk(...);
    }
}

public class DiskReader {
    ReaderUtil diskReader;

    public DiskReader() {
        diskReader = new ReaderUtil(...);
    }
    public void doSomething() {
       int space = diskReader.getAvailableSpace(...);
    }    
}

This solution prevents classes from having access to methods they should not, but it also breaks encapsulation. The original DiskHandler class was completely self-contained and only needed config parameters via a single constructor. By breaking apart the functionality into read/write classes, they both are concerned with the directory and both need to be instantiated with their respective values. In essence, I don't really care to duplicate the concerns.

Solution 2

I could implement an interface which only provisions read operations, and use this when a class only needs access to those methods.

The interface might look something like this:

public interface Readable {
    int getAvailableSpace();
}

The Reader class would instantiate the object like this:

Readable diskReader;
public DiskReader() {
    diskReader = new DiskHandler(...);
}

This solution seems brittle, and prone to confusion in the future. It doesn't guarantee developers will use the correct interface in the future. Any changes to the implementation of the DiskHandler could also need to update the interface as well as the accessing classes. I like it better than the previous solution, but not by much.

Frankly, neither of these solutions seems perfect, but I'm not sure if one should be preferred over the other. I really don't want to break the original class up, but I also don't know if the interface buys me much in the long run.

Are there other solutions I'm missing?

---

I'd go with the interface, combined with a little bit of Dependency Injection - you don't instantiate a new DiskHandler directly inside your reader or writer classes, they accept an object of the appropriate type in their constructors.

So your DiskReader would accept a Readable, and your DiskWriter would get a ReadWrite (or a DiskHandler directly, if you don't want to make an interface for the read-write mode, although I'd suggest otherwise - via interface ReadWrite extends Readable or similar). If you consistently inject it using the appropriate interface, you won't have to worry about incorrect usage.

---

I think the interface is also the most object-oriented approach here. The first approach basically refactors your collection of semantically-related methods into a bunch of little utility functions: not what you want.

The second solution allows users of your class to express exactly why they are using it. In the same way that good Java code typically declares List, Set, and NavigableMap rather than ArrayList, HashSet, and TreeMap, users of your class can declare a variable to be only a Readable or Writeable, rather than declaring a dependency on any concrete subclass.

Obviously, someone still needs to call new at some point, but as tzaman pointed out, this can be handled with setters and dependency injection. If you need an unknown number of them at runtime, inject a factory instead.

I am curious: why do you think that any changes to the implementation of DiskHandler would result in changes to the classes that use Reader? If Reader can be defined to be a stable interface, the interface should clearly spell out its semantic contract (in the Javadoc). If users code against that interface, the implementation can be changed behind the scenes without their knowledge. Sure, if the interface itself changes they have to change, but how is that different from the first solution?

One more thing to think about: Let's say you have multiple threads, most of which need a Reader, but some of which need a Writer, all to the same file. You could have DiskHandler implement both Reader and Writer and inject a single instance to all threads. Concurrency could be handled internally to this object by having appropriate ReadWriteLocks and synchronizeds where they need to go. How would this be possible in your first solution?