Singleton is an anti-pattern

Seven things I really hate in database design

1. Common prexif in all table names
1. es: TXXX, TYYY, TZZZ, VAAA, VBBB - T stays for Table, V stays for View
2. es: APPXXX, APPYYY, APPZZZ - APP is an application name
2.  Common prefix in all field names in every table
        es: APPXXX.XXX_FIELD_A, APPXXX.XXX_FIELD_B, APPXXX.XXX_FIELD_C
3. Fields with the same meaning and different names (in different tables)
        es: TABLE_A.BANK_ID, TABLE_B.BK_CODE
4. Fields with the same logical type and different physical types
         es: TABLE_A.MONEY_AMOUNT NUMBER(20,2)
         TABLE_B.MONEY_AMOUNT NUMBER(20,0) -- value * 100
         TABLE_B.MONEY_AMOUNT VARCHAR(20) --value * 100 as char
5. No foreign-keys nor integrity constraints at all - by design
6. Date (or generally structured data type) representation with generic and not specific types
1. es: TABLE_A.START_DATE NUMBER(8,0) -- yyyyddmm as int
2. es: TABLE_B.START_DATE VARCHAR(8) -- yyyyddmm as char
7.  (possible only in presenceof 6.) Special values for semantic corner-cases which are syntactically invalid
         es: EXPIRY_DATE = 99999999 -- represents "never expires case",
      but... IT'S NOT A VALID DATE!!! - why not 99991231??

Exception management antipatterns - Episode I

Exception management is a tricky skill, and perhaps the most misunderstood topic of Java - and I think not only Java - programming. So, I will try to collect a series of posts about antipatterns in exception management: to provide a reference to anyone interested in the topic, of course, but also to analyze and deepen the issue myself.

My first post on the subject concerns a number of basic antipatterns:

• Throw all away
• Empty catch
• Re-throw without cause
• Log and rethrow

Throw all away

This is the simplest exception management antipattern we can (and shouldn't!) use, and is the tipical approach used when studying a new language: no exception management! While this can be a legitimate approach in training contexts - I'm learning Java IO API and will initially concentrate on the sunny day path, not on exception management - or in testing contexts - I'm writing a test that doesn't cover exception handling: another test will cover it! - it's not admittable in normal, production code... simply because without exception management one exception causes program termination.

public void aMethod() throws Exception {

   doSomething();

}

public void callerMethod() throws Exception {

   aMethod();

}

public void callersCallerMethod() throws Exception {

   callerMethod()

}

...

public static void main(String[] args) throws Exception {

    callersCallerscallersCallers...CallerMethod();

}

etc. etc. etc. ... when doSomething method raises an exception, it goes up through the entire stacktrace and causes program termination. Oh!

Empty catch

This is the antipattern which causes more headaches, as it hides exceptions instead of really managing them: unlike the previous antipattern, Empty catch makes very difficult bug finding and fixing, since it gives no information about the exception taht has occurred: the exception simple disappears, the current operation is not completed successfully, the user is not given any warning about the unespected application behaviour, and the developer has no useful information for analyzing the issue: great!

public void aMethod() {

   try {

      doSomething();

   }

   catch(Exception e) {

   }

}

Re-throw without cause

Another exception management antipattern I've often seen at work is the Re-throw without cause one: exceptions are catched, exception handling code constructs another, tipically custom, exception and throw it... without reference to the initial exception.

This can be useful in some specific contexts, such as code in layers that are called using a remote protocol: if you do not want to expose your internal exception through some type of serialization mechanism (RMI, SOAP, ...), you can throw another interface (known to the client) exception: this newly created exception exposes the problem in the client language.

In regular code, however, creating and throwing and exception without reference to the cause exception is something like Empty catch: it hides the real problem and causes headaches, both to users and debuggers, since it does not give sufficient information to analyze the problem.

public void aMethod() throws ABCException {

   try {

      doSomething();

   }

   catch(XYZException e) {

      throw new ABCException("Error!");

   }

}

Log and rethrow

This antipattern refers to both exception and logging management: it consists in catching an exception for the only purpose of logging it, and then rethrow it (possibly wrapped in another). This is an antipattern because it causes a stack-trace pollution in log file, wich makes more difficult to read and interpret it. Try to search debugging information in logs of an application which common exception management approach is Log and rethrow: you'll find a series of repeated stacktraces, one for each exception; look for significant informations in such a mess is more and more difficult than look for them in a repetition-free log file. Such an approach increases log size without adding significant information, with the effect of diluting thesignificant content.

public void aMethod() throws AnException {

   try {

      doSomething();

   } catch(AnException e)  {

      logger.log("Error!", e);

      throw e;

  }

}

public void bMethod() throws AnException {

   try {

      aMethod();

   } catch(AnException e)  {

      logger.log("Error!", e);

      throw e;

  } 

}

public void cMethod() throws AnException {

   try {
      bMethod();
   } catch(AnException e)  {
      logger.log("Error!", e);
      throw e;

  }

}

 

public void dMethod() throws AnException {
   try {
      cMethod();
   } catch(AnException e)  {
      logger.log("Error!", e);
      handleException(e);

  }

}

 

In this example the AnException is logged four times before it is actually handled, and the log is four times more verbose than necessary - and four time less readable...

Improper inheritance and light-hearted downcast

And here is maybe the worst design I've ever seen - directly from production-released code, of course...


Inheritance is a key feature in object-oriented programming languages, providing support for both specialization and flexible extensibility. But... using inheritance just to use it can drive to horrible design cases, where distinction between superclass/interfaces and different subclasses is improper and extensibility is actually impossible. This is the case of a real-life production code I was lucky enough to see:

public interface InterfaceX {
   public abstract void aMethodX();
}

public interface InterfaceY {
   public abstract String aMethodY(final String s);
}

Class FatImplementation implements both InterfaceX and InterfaceY:

public class FatImplementation implements InterfaceX, InterfaceY {
   public void aMethodX() {
      ...
   } 

   public String aMethodY(final String s) {
      ...
      return ...;
   }
}

And... Ladies and Gentlemen... the crazy client class, whose method is casting to InterfaceX an InterfaceY reference, relying on the fact that FatImplementation implements both interfaces:

public class CrazyClass {
 public void theCrazyCastingMethod(final InterfaceY y) {
  ...
  ((InterfaceX)y).aMethodX();
 }
}

This is an improper use of abstraction - inheritance programming model, since the cast succeeds only with a specific implementation (FatImplementation) of given interface and polymorphism is actually not possible.

... ... ... poorly and foolishly designed, isn't it? And... what if I said that InterfaceX and InterfaceY, in the original code, where StatefulXXX and StatelessXXX? So... FatImplementation was stateful and stateless at the same time! AAAAAAHHHH!

Singleton, testing and dependency inversion

Singleton: pattern or antipattern?

Accordingly to Wikipedia, Singleton is a creational pattern, used to implement the mathematical concept of a singleton, by restricting the instantiation of a class to one object. So, it's a pattern!
But it's an antipattern, too, especially from the point of view of testing: it's a (simple) variant of Service Locator testability antipattern.

As states Jens Schauder in his post Fixing the Singleton, there are two key characteristic of the (classic implementation of) singleton:

• There can be only a single instance of the class developed as singleton
• There is a central, global acces point to the singleton instance

Alhough the first one is the main - if not the only - reason for use of Singleton pattern, it comes almost alway with the second one. But... while the first one is a conceptual featur of the pattern, the second is nothing but an implementation detail!

We can therefore speak of conceptual Singleton, when we have a class that can be instantiated only once in the application lifecycle, and syntactic Singleton, with reference to the traditional GoF's implementation. 

Well, my idea is that you can think of two different basic implementation strategies for conceptual Singletons:

• Singleton by syntax - traditional GoF's implementation, through private static instance and public static (and then: global) accessor
• Singleton by contract / application - implementation of the concept of "single class instance" without syntactic constraints: application code takes care of respect the contract of "single instance". Tipically, application infrastructure responsible for creating object and setting up collaborators references instantiates the Singleton class only once and passes created instance to modules interested in its use: this is substantially an application of Dependency Inversion Principle, and can be implemented through Inversion of Control frameworks like Spring and Google-Guice (for a good discussion about self implemented dependency injection, see this article).

First approach suffers the problem suggested initially: there is a global state, publicly accessible, liberally referenced everywhere in the client code - and global state is evil!

The second one, instead, provides a conceptual Singleton instance without referring to syntactical constraints: application lifecycle infrastructure ensures unicity of the Singleton class instance.

In code:

• Singleton by syntax:
  package singleton;

public class UIDGenerator {
  private static final UIDGenerator INSTANCE = new UIDGenerator();

  public static UIDGenerator getInstance() {
    return INSTANCE;

  }

  private UIDGenerator() {
  }

  public String nextId() {
    return ...;
  }
}
  Client code:
  
  public void foo() {
  String newId = UIDGenerator.getInstance().nextId();
  // Use newId
}
  
public void bar() {
  Account account = new Account(UIDGenerator.getInstance().nextId());
  // Use newly created Account
}
This the classical GoF's implementation of pattern Singleton: private constructor and final static INSTANCE ensure instance unicity, public static accessor provides global access to singleton instance.
• Singleton by contract:
  package singleton;

public interface UIDProvider {
  public abstract String nextUid();
}
  
  Client code:
  
  package singleton;

public class AccountManager {
  private final UIDProvider uidProvider;

  public AccountManager(UIDProvider uidProvider) {
    this.uidProvider = uidProvider;
  }
  
  public void bar() {
    Account account = new Account(uidProvider.nextUid());
    // Use newly created Account
  }
}

In the second implementation we define an interface for UID generation: application infrastructure (i.e.. in most cases. an Inversion of Control container, like Spring) will ensure that a single instance of a class implementing UIDProvider is passed whenever it's necessary.
This way we can obtain the conceptual part of the pattern without the syntactical one: there is no public static context accessed everywhere, and a reference to the singleton is indeed injected into modules that need it. So, unlike in the first case, it's possibile to mock UIDProvider for testing purposes (for example because real implementation is time expensive, or there is a fee for every use, or simply because unit testing is isolation testing and we need to make assumptions on generated uid in testing code):

public class AccountManagerTest {
  @Test
  public void testMethod() {
    AccountManager underTest = new AccountManager(new FixedUIDProvider());
    // Exercises underTest
  }
}

This is IMHO a more, more (more!) powerful approach for implementing singleton than the classic one: can you figure out how to mock UIDGenerator.getInstance().nextId() calls?
The basic idea behind this proposal is a variation of single responsibility principle: classic singleton implementation drives to classes that implement two responsibilities: a functional responsibility - what the class do - and a structural responibility - how is the class instantiated and the instance accessed. Inversion of Control containers, and more generally the idea of Dependency Inversion, support separation of responsibilities by divide functional code and object graph lifecycle management code: this leads to clearer, simpler design, that decouples singleton implementations from client modules and supports testing in a more effective way.

Configuration architectural antipatterns

I've got the flu, these days: and in fever's drowsiness has surfaced the memory of an application I've worked on six years ago.

It was an aged application, developed by a former employee using Visual Basic 6 - uh! oh!

I was doing only fixes and small enhancements on that application, but I'll remember it forever due to its bright and genial configuration-relating design.

That application's configuration, as was the case also for other applications by the same author, was "designed" around the Multiple Configuration Points conceptual antipattern: it wasn't stored in a single, well defined, simple accessible place, but in multiple, differently implemented respositories:

• part of the configuration was read from an "ini" file: the file was processed without using some kind of library, but reading the raw input and counting rows: at row #X, parameter Y - oh, yeah!
• the remainder part of configuration cames from a MS Access File (may god forgive him! - if a god exists) and was accessed using OLE DB support and SQL queries (this part of the configuration was modifiable through the application itself)

So, in one application, and only with regard to configuration management, you can list several architectural antipatterns:

1. Multiple Configuration Points: configuration spawned in multiple, heterogeneous repositories
2. Reinventing The Wheel: "ini" file read through home-made procedure
3. Brittle configuration: formatting-depandant "ini" file semantics
4. Tightly coupling to external libraries versions: MDB configuration file could be read without errors  only when the MS Access' version on the host machine was the version for which the application was compiled - otherwise, the application wasn't ever able to start. And if the MDB file was modified using a most recent version, it became unreadable for the application...

And - final masterpiece: that application, which used relational DB for (partial) configuration management, did not use any type of database for business data storage and inter-application communication purposes: data storage was performed writing sequental text files with fixed-length records, and applications interested in data interchange read the same files (code reading and writing records was naturally duplicated between applications, not extracted in a common library).

I've got the flu, these days... but even conceive of such an architecture must have required a great fever... isn't it?