💎 More abstractions

[wxxedu]

Hey guys I am back, keep over-engineering things.

Building on top of my response to issue #23, there are a few things that I wish could be better, one of them is that currently for my output, I used System.out.println. While there is no real issue with such an approach, it would not be as flexible as I would like it to be, in that:

1. What if I were to change the output? Say, if I were to change the output to writing to a file? Or how about a gui?

2. With System.out.println, it is quite hard to conduct unit tests, at least I don’t know how. Maybe I will have to mess with the output streams? (I am not a Java expert, and my past experiences have mostly been working with Dart / Flutter) Also, I hate bash scripts. You cannot have empty spaces around the assignment operator, what?

Writable

Clearly, the way to solve this problem is through abstraction. Hence, I have created a new Interface called Writable.

public interface Writable {
    void write(Object object);

    void writeln(Object object);
}

Now, with this, I can put a field called Writable in the classes that I wish to write. For example, the Echo class from [here](https://www.notion.so/On-my-design-of-Project-Duke-9860c58274714de3a50a076250a21e59).

public class Echo implements IdentifiableExecutable {
    public Echo(Writable writable) {
        this.writable = writable;
    }

    private final Writable writable;

    @Override 
    public String getId() {
        return "echo";
    }

    @Override
  public ExitStatus execute(String[] tokens) {
    System.out.println(String.join(" ", tokens));
    // after this, I would want it to skip the current execution loop.
    return ExitStatus.finishCurrentIteration;
  }
}

Now, I can replace all occurrences of System.out.println with writable.writeln.

SystemOut

Since Writable is only an interface, I do still need concrete implementations. I would want to create an implementation that wraps around the System.out.println. Hence I just call it SystemOut:

class SystemOut implements Writable {
    @Override 
    public String write(Object object) {
        System.out.print(object.toString());
    }

    @Override 
    public String write(Object object) {
        System.out.print(object.toString());
    }
}

This way, in the instantiation of my Echo class, I could just pass in a new SystemOut instance.

Clean Architecture

Now, with the System.out.println removed from our code, I have successful abstracted away a very “specific” feature from the core of my application logic. With that, I can do some reorganization of the files according to the clean architecture proposed by uncle bob:

Clean Coder Blog

Essentially, I want to put the application logic at the core of our app, and make sure that no dependencies of the outer rings can be found inside the app. More specifically, I created a folder called presentation, which shall contain the class SystemOut and the DukeEventLoop. The other classes, such as Writable, Executable, EventLoop (super class of DukeEventLoop), shall be placed inside the entities folder, and the implementations of Executable's, i.e. Echo, Bye, TaskManager, got renamed to EchoUsecase, ByeUsecase, and TaskManagerUsecase, and shall be put inside the usecases folder. I grouped the usecases and the entites folder into a larger folder called domain. Ideally, regardless of how you changed the presentation or the database, you should not change the domain folder.

Factories

Using the Writable interface has mostly solved the second problem,

because now I can write a mock writable class that verifies the behaviors of my code.

Factory Methods for Instantiation

However, it still failed to fully addresss the first problem. The matter is that if I pass in a new SystemOut instance every time I instantiate a new class, if I were to switch to a different implementation of the Writable interface, it would still require me to go out and search for all the SystemOut occurences, and swap them by hand. The most ideal way, however, is to use a factory to generate new SystemOut instances. This way, if I were to change the SystemOut to a new implementation, say WriteToFile, then I would only need to change the instantiation in the factory. All the other code gets kept!

// ideally:
Writable getWritable() {
    return new SystemOut();
}
// and replace all the new SystemOut with invocations of getWritable().

However, there are two issues with this approach:

1. Java does not support top level functions, so it has became a problem as to where to put this getWritable method. To me, the Writable class seems to be a place where this makes the most logical sense. However, since Writable is considered an entity, as described in the clean architecture, we cannot have dependencies outside of it. (For the Writable class to create a new SystemOut instance, we will have to import the file for SystemOut, which violates the dependency rule of clean architecture).
2. If we have 10 more similar classes with features, we would have to write 10 similar functions, which is in a sense, code duplication.

Singletons

Also, observing from the characteristics of the Writable, we can also find one thing: we do not need many instantiations of the Writable object: having only one object would be good enough for us, and having many could potentially waste some resources. Therefore, in the end, we would want:

1. A factory that can provide an instance of any class to our liking;
2. The instance that it provided, regardless of when it was called, is exactly the same instance, i.e. we want to create singletons for the sake of saving resources and having a single source of truth for some stateful instances.

/**
 * The utilities class for handling the registering singletons. To use it, call
 * <code>Singletons.registerSingleton(Hello.class, new Hello());</code>
 * This way, it can provide a new layer of abstraction, and if any concrete
 * implementations were to change, any code calling <code>Singletons.get</code>
 * would not need to be changed, unless, or course, you directly call the
 * class itself.
 */
public class Singletons {
    /**
     * The dependencies stored in this dependencies section.
     */
    static private final Map<Class<?>, Object> singletons = new HashMap<>();

    /**
     * The dependency suppliers, used for lazy registration.
     */
    static private final Map<Class<?>, Supplier<?>> lazySingletons =
            new HashMap<>();

    /**
     * Registers a class as a singleton, therefore, it would only be created
     * once.
     * @param cls the class of the object.
     * @param object the object itself.
     * @param <T> the type of the object.
     */
    static public <T> void registerSingleton(Class<T> cls,
                                             T object) {
        if (singletons.containsKey(cls) || lazySingletons.containsKey(cls)) {
            return;
        }
        singletons.put(cls, object);
    }

    /**
     * Registers a class as a lazy singleton. A lazy singleton will only get
     * instantiated if it was used.
     * @param cls the class of the object
     * @param supplier the lazy singleton of the object.
     * @param <T> the type of the object.
     */
    static public <T> void registerLazySingleton(Class<T> cls,
                                                 Supplier<T> supplier) {
        if (singletons.containsKey(cls) || lazySingletons.containsKey(cls)) {
            return;
        }
        lazySingletons.put(cls, supplier);
    }

    /**
     * Gets an instance of the specified class.
     * @param cls the class of the class.
     * @return the object that has been registered for this class.
     * @param <T> the type of the object.
     */
    static public <T> T get(Class<T> cls) {
        if (singletons.containsKey(cls)) {
            return cls.cast(singletons.get(cls));
        } else if (lazySingletons.containsKey(cls)) {
            final T object = cls.cast(lazySingletons.get(cls).get());
            singletons.put(cls, object);
            lazySingletons.remove(cls);
            return object;
        }
        throw new RuntimeException("Dependency for " + cls.getName() + " has " +
                "not been injected.");
    }
}

There are a few notes here:

• I am not a Java developer, and I don't know what is the common practice for this in Java, so take this with a grain of salt.
• The Class of the type is used as the key.
• I allow for registering of both singletons and lazy singletons, and the latter one is lazily evaluated. Once evaluated, it would be put inside the singletons set, and the corresponding supplier would be removed from the lazySingletons set.
• The reason that I throw a RuntimeException instead of a checked exception in the last method is that this could only be wrong if the developer forgot to register the corresponding dependency, and it can be fixed programmatically. Hence, it should not be checked.

This way, in the Duke class, I could write a static method called configureDependencies():

private static void configureDependencies() {
    Singletons.registerLazySingleton(Writable.class,
        () -> new SystemOut());
    // ...
}

and call this line before executing the rest of the code.

Therefore, I could replace all the occurrences of instantiating a new Writable with this:

Singletons.get(Writable.class);

[damithc]

Thanks for sharing this @wxxedu Probably a bit too much for most classmates at this time, but as the codebase gets bigger over the next few weeks, they will be in a better position to reflect on these aspects.

On a side note, overengineering is not a good thing in real projects but is useful in learning. For example, by pushing into the realm of overengineering one can learn where the boundary of overengineering is.

[Zxun2]

This is really insightful and interesting. Thanks for sharing! @wxxedu