Tuesday, July 19, 2016

Two Tips to Immediately Improve the Quality of Your Code

* While this post was written mainly with Java in mind, at the great peril of appearing to "Language Map", the concepts are certainly transferable across a great many languages, particularly those supporting OO semantics.

Stop Writing "public"

Think fast! What's the first thing you write when creating a new interface or class? Or, if your IDE has a template for producing this kind of boilerplate, what does the resulting code typically look like? I'm willing to bet it's something along these lines:

public class Foo {
    ...
}

Is this more or less the standard template that you (or your IDE) consistently regurgitate? I myself carried on like this without so much as a second thought for awhile before it became clear to me what a poor design decision it was, conscious or not. When everything is public:
  • As the producer… 
    • You (potentially) lose out on the opportunity to channel consumers into the pattern(s) of usage you intended for with your design.
    • You (potentially) leak implementation details that you really don't want consumers to develop a reliance upon.
  • As a consumer… 
    • Short of explicit samples or documentation, there isn't a clear indication as to how you're expected to engage.
Rather than blindly making everything public, think instead in terms of "modules".  A well-designed module will have a contract for consumers to make use of it, while simultaneously shielding them from internals that aren't meant for consumption outside of the module itself. Therefore, the visibility modifier for each class/interface should be determined through careful deliberation about where it should fall along this spectrum. Stop and ask yourself "Why should this be public?"

As an example, imagine a data access module that offers the flexibility of abstracting away data access by way of a generic Repository interface. The Repository allows us put() and remove() instances into/from storage respectively, while using a consumer-supplied implementation of a SelectionPredicate interface for instance fetching that allows retrieval to be fine-grained and context-specific. Internally, the module has Repository implementations for both JDBC-compatible databases as well as MongoDB, which are provided by way of a factory class (without exposing the implementation classes themselves). One way of crafting such a module might be a package like the one below:

repository
├── public interface Repository
├── public class RepositoryFactory
├── public interface SelectionPredicate
├── protected class JdbcFooRepositoryImpl
└── protected class MongoFooRepositoryImpl

Here, the things that are meant to be public are modified as such without the implementation details leaking out beyond the boundaries of the package. This opens up a path to comfortably evolving the internal implementation without disruption to consumers, while also gently nudging those consumers into a straightforward, obvious pattern of usage.

In short - the use of public should be a conscious and deliberate design decision, not simply a byproduct of habit.

Stop Writing "new"

The new operator of course instantiates a class by allocating heap memory for a new object instance, and returns a handle referring back to that bit of memory. While seemingly straightforward, widespread usage of new has design consequences that can be difficult to foresee, especially when working in smaller codebases. Namely:
  • Overuse can make it more difficult to reason about the relationships between abstractions. Invoking new reveals a direct dependency on the type being instantiated, and thus the calling code must know, in addition to its own business logic, the intimate details behind how to construct that dependency, and that dependency's dependencies, and that dependency's dependency's dependencies, and, and, and… As the codebase grows, the code will become increasingly sensitive to change and more difficult to distinguish between the parts that perform the business logic and the parts that simply bootstrap other dependencies.
  • It eliminates test seams. When classes initialize their own concrete dependencies, we lose the opportunity to programmatically swap out implementation types, which is insanely useful for the purposes of testing. If, for example, a component is responsible for performing database lookups, and as part of its initialization explicitly establishes a connection to its target database, there is no way to test that component in isolation. There is no way to supply a test double such as a mock database for the purpose of programmatically testing the component under different conditions (i.e. no items found, one item found, many items found, etc).
Instead of invoking new directly, components should have dependencies on abstractions, and instances of those abstractions should be provided as needed (e.g. at instantiation time, per invocation, etc). The following snippet illustrates the above points:

interface Widget {}

interface WidgetDao {
    public Collection<Widget> getWidgets();
}

class WidgetPrinter {
    private final WidgetDao widgetDao; 

    WidgetPrinter(WidgetDao widgetDao) {
        this.widgetDao = widgetDao;
    }

    public void print() {
        for(Widget widget : widgetDao.getWidgets()) {
            System.out.println(widget);
        }
    }

}

Here, WidgetPrinter is simply given a data access abstraction that can return Widget instances. Rather than constructing that dependency directly, that job is delegated elsewhere and the result is simply injected into the WidgetPrinter and used. In a testing context, we could inject some customized variation of the WidgetDao interface into WidgetPrinter that, for example, returns a hard-coded set of Widget instances. As a result of applying this pattern, the relationships between the abstractions become clearer, and there is a clear path to testing components.

No comments:

Post a Comment