Article 4VP75 Newly Singleton

Newly Singleton

by
Remy Porter
from The Daily WTF on (#4VP75)

Shery was brought on to help with a project which was "going well". "Going well" meant that all the little icons on the project management dashboard were green, which is one of the most effective ways to conceal a project which is struggling.

Shery's component was large, and it was complicated, and it had a well defined interface to the rest of the application. Specifically, they had a documented JSON message format which her code would receive via JMS. That meant she could do the vast majority of her work in isolation, without talking too much to the existing team, so she did.

But the day came when they were going to send her messages. And the very first message her code received was a JSON document which looked like this: {}.

That wasn't correct. Shery's code dutifully handled and logged the exception, and she took it on herself to diagnose the problem. She pulled up the code from the other part of the team.

The first thing Shery noticed was all the code copy/pasted from StackOverflow. She could tell it was copy/pasted because that was the only code with any sort of sane indenting. All the code developed in-house used indenting stochasticly. One group of developers had clearly turned off their autoindenting in the IDE, another group hadn't, and the result was a mishmash.

Most of the code was clearly done via copy/paste. If someone wrote a block of code in one section of the application, and someone else needed that functionality, they'd just copy/paste it around. There were miles of unused imports at the top of pretty much every file, there were statements following the pattern if (someCondition) { } else if (theExactSameConditionAsTheIf) { }. Suffice to say, there were nearly as many warnings as there were lines of code.

Shery decided she wasn't going to debug or alter their code. Instead, she raised the issue she was seeing- empty messages- and politely suggested that she had noticed some "non-compliance" with the company's coding standards which should probably be addressed, at some point.

While she was busy looking at the other team's code, someone from the other team was looking at her code. And when she checked source control, there was a fresh commit at the head of the branch where they "fixed" some of her issues.

Shery had an object which provided a service. This object was itself stateful. That state should be the same everywhere in her component. So Shery created a Singleton.

Setting aside the concerns of managing any sort of global state, even in a Singleton, they were doing this in Spring. Spring, like most Java containers, has all sorts of features and functionality to manage object lifecycles. In "pure" Java, if you wanted a Singleton, you might do something like this:

public class Singleton { private Singleton() {} private static Singleton instance; public static Singleton getInstance() { if (instance==null) { instance=new Singleton(); } return instance; } public void doSomething() {...}}

It's a load of boilerplate, but now you can interact with it by calling: Singleton.getInstance().doSomething(). No matter where you are in the application, you'll always be interacting with the same instance.

Java frameworks, like Spring, exist, in part, to hide this boilerplate from you. Spring lets you make components, and assumes that the default scope for that component should be singleton, so this:

@Componentpublic class Singleton { public void doSomething() {...}}

Is the same as before, but with a lot less garbage code.

Even better, if a class wants an instance, they can define the property like this:

@Autowiredprivate Singleton svc;

At runtime, the container will managed the instance, and when anyone requests it (using decorations like @Autowired) the container will inject the correct instance. This is the "right way" to do this sort of thing in Spring.

When Shery checked the commit her peers had written, they had done this instead:

@Componentpublic class Singleton { private Singleton() {} private static Singleton instance; public static Singleton getInstance() { if (instance==null) { instance=new Singleton(); } return instance; } public void doSomething() {}}

In essence, they mixed the pure Java approach and the Spring approach to accomplish nothing. For bonus points, they kept the autowired svc variable, but would interact with it like this:
svc.getInstance().doSomething().

Shery never got a chance to have a conversation with the developer responsible for that change. The project continued to "go well", you see, so well in fact that half the team was sacked and the project was moved to another department to be taken to completion. Don't worry, the dashboard continued to show green for all of their key-performance indicators.

proget-icon.png [Advertisement] ProGet can centralize your organization's software applications and components to provide uniform access to developers and servers. Check it out! TheDailyWtf?d=yIl2AUoC8zA5irYST8y5E8
External Content
Source RSS or Atom Feed
Feed Location http://syndication.thedailywtf.com/TheDailyWtf
Feed Title The Daily WTF
Feed Link http://thedailywtf.com/
Reply 0 comments