Moved!

We’ve moved! Our new site here. And our RSS here.

Seeya there!

Anemic Domain Model Antipattern

Anemic ¿Qué!??

Hoy toca hablar de patrones, o mejor dicho, de anti patrones. Esas cosas que se deben evitar por norma general.

Mucha de la gente que venimos de lenguajes procedurales (vaya palabro…) tipo C, tendemos a pensar de esta forma a la hora de programar en Java. ¿Por qué? Por que C no es un lenguaje orientado a objetos. Para representar en C un dato usamos lo siguiente:

typedef struct point { int x,y; } point;
....
y usamos punto.x
Así que cuando vamos a un lenguaje orientado a objetos tendemos a hacer cosas de este tipo:

public class Casa{
private String nombre;
private boolean alquilada;
private boolean habitable;
public void setNombre()...
public String getNombre()...
public void setAlquilada()...
public boolean isAlquilada()...
....
}

public class Persona{
private String nombre;
private List casas;
public void setNombre()...
public String getNombre()...
public void setCasas()...
public List getCasas()...
}
..
List casasNoAlquiladas= new ArrayList();
for(Persona persona: personas){
for (Casa casa : persona.getCasas()){
if (! casa.isAlquilada() && casa.isHabitable()){
casasNoAlquiladas.add(casa);
}
}
}


….

¿Cuál es el problema con esto? Pues que estamos utilizando las clases de negocio como simples estructuras de datos. Hemos avanzado en la tecnología, ¡ Avancemos en su uso también! Vale, siempre lo has hecho así, te sientes cómodo con esta solución ¿Con qué problemas te vas a encontrar?

• Lógica de negocio dispersa por toda la aplicación, seguramente incluso repetida.
• Complejidad a la hora de hacer las clases controladoras
• Estamos exponiendo la forma en que mantenemos las casas al exterior. Solo debemos exponer las entities, no los value objects! (esto daría para otro post)
• ¿La lista de casas que devolvemos es inmutable? ¡Cuidado!
• Sumando todo lo anterior: Más complejidad = más bugs, además de menos extensible

Bueno y ¿Cómo lo solucionaríamos? Muy fácil, introduciendo la lógica de negocio en las clases de negocio:

public class Casa{

….

private final String nombre;
private boolean alquilada;
private boolean habitable;
public String getNombre()...
public void setAlquilada()...
public boolean isAlquilada()...
public boolean isAlquilable(){
return (! casa.isAlquilada() && casa.isHabitable());
}

….
}

public class Persona{
private String nombre;
private List casas;
public void setNombre()...
public String getNombre()...
public void setCasas()...
public List getCasasAlquilables(){
List list= new ArrayList();
for (Casa casa: casas){
if (casa.isAlquilable()) list.add(casa);
}
return list;
}
}
..
List casasNoAlquiladas= new ArrayList();
for(Persona persona: personas){
casasNoAlquiladas.addAll(persona.getCasasAlquilables());
}

Más fácil ¿Verdad? Si no conocías este error común, seguro que ahora te vienen a la memoria un montón de casos en los que podrías ahorrar mucho en código y hacerlo mucho más extensible.

Otro ejemplo: En cierto cliente, hace un cierto tiempo y haciendo cierta aplicación 🙂 De una clase muy importante en el modelo (Entity) sacabamos unos cuantos informes. En un momento dado, me pidieron que si se podía cambiar cierta lógica a la hora de generar los informes. Mi respuesta fue: tardo 2 segundos (y uno para pulsar ctrl+shif+T en Eclipse!! ). ¿Por qué? Porque mi clase de ese entity tan importante tenía lo siguiente:

List<Dato> getSubObjetosQueCumplenCiertaCondiciónDeNegocio(){
for....{
if (obj.condicion1() && obj.condicion2() && entity.....)

lista.add(...)
…

}
}


Lo que me pedían era un cambio de negocio. Yo había entendido mál, un objeto de tipo Dato debía de cumplir obj.condicion() && obj.condicion3(), pero el cambio no suponía el menor esfuerzo. ¿Cuánto hubiese costado si hubiesemos tenido este código disperso por toda la aplicación?

Se puede discutir si incluir los DAO dentro del objeto de negocio es bueno o malo (Rails y Grails lo hacen), si algúna lógica concreta debería ir en los controladores, pero al menos este tipo de cosas creo que pueden ayudarte bastante.

Bueno, no se si ha quedado claro, hoy no he dormido mucho y estoy muuuuuy cansado 😀

You Need Code Reviews

Look at this code:

(within a while body)

...
if (log.isDebugEnabled()) log.debug("Parsing line " + (lineNumber++));
...
lineNumber used here
...

You might be wondering: what´s wrong with it? Well, it turns out that this sort of construction it could become extremely difficult to spot in certain conditions. Imagine a hypothetical situation where this code is not working properly in the production environment, and meanwhile the developer in charge of this block of code -the guy as cool as a cucumber- is totally sure about the correctness of it. Okay, you got it, you may be thinking of me: “How noob this guy is!”; but Hey!, sometimes you are toiling late in the night and this kind of bugs pass unnoticed until you receive an email from your Development Manager muttering:

Subject: Potential bug

I have come across with this statement –incidentally- while surfing the code. Beware that the log will be printed out only if the log level is set to DEBUG, so the lineNumber variable won´t be incremented.

All the best,

Your Development Manager

Guess which level the Production Support had established in their environment? Fortunately, I started off saying “Imagine a hypothetical situation …“.

Share your knowledge

Thomas Jefferson said:

And no matter how many people share it, the idea is not diminished. When I hear your idea, I gain knowledge without diminishing anything of yours. In the same way, if you use your candle to light mine, I get light without darkening your. Like fire, ideas encompass the globe without lessening their density.

Sometimes, in this hectic world, you realise that your knowledge about a subject has increased considerably, and probably at that moment you are an expert on it. The “problem” now is how you ought to handle that situation in regards to your team mates. You can keep such expertise for your own benefit, considering them a sort of nuissance, if not worse. But that is not supportive. As I see it, helping your co-workers is a professional duty, no matter the reason. No matter the guy. You must do it because you must do it. And that´s all. There shouldn´t have to exist more reasons besides that one. Yeah, I know, there are lotsa situations where excuses arise in order not to help someone, but anyway I think it is a good principle to start with. Write it down in your tickler file.

Nonetheless, there are some important benefits of being supportive and willing to give others a hand:

1. You can cause a great impact in the morale of your huddle, because your positive attitude might make them want to better themselves in turn. The overall competence of your team spikes. Betcha! That´s good for you as well. If this doesn´t work, pull out.
2. You may think you are an expert. Believe me: even experts are requested to answer questions they don´t know about! IMHO, I consider this awesome. The search for those answers permits you to assimilate unexplored areas of the topic on discussion. Therefore, you are gaining more insight, which is always neat. For example, the other day someone asked me about subversion, and how this system performs the merging. I thought I had that under control. I explained to him the basics of the merging in subversion, the reason it should be called diff-and-apply instead of merging, how two revisions from the origin and destination trees are applied to the working copy, and so on. So far, so good. All the same, point in time, he asked me how a merge was actually applied to a specific file in the working copy. That was kinda going thorugh bad times, because I didn´t understand it seamlessly, hence I wasn´t able to provide him with an answer. Well, in fact, I had to look up that section in a subversion book, read it thoroughly, sink in all that stuff, and lastly give to him the right answer. I could have told him to grab a book and look it up. That would have been a lazy behaviour and, what it´s much worse, I would still be thinking I have that part under control now. It´s wonderful not to be an expert in nearly anything, isn´t it?
3. Corollary: the rest of the team come to trust they can rely on you when they feel they´re dead in the water.

Corollary to the last Corollary: Be nice 😉

AbstractTransactionalSpringContextTests meets TestNG

Introduction

In the search of an alternative to JUnit, in relation to the testing of our classes, we -Mikel and I- decided to give TestNG a try. This testing framework takes advantage of a useful feature introduced in the release 5.0 of the Java Language: the annotations. In addition, it provides a rich set of facilities to ease the unit and integration testing of Java classes, like groups and data providers. Unfortunately, these are very well explained in the home page of the project, so let´s consider them as being out of the scope in regards to the main thread of this post.

The Problem

What we want to have: test cases running within a transactional context AND take advantage of the TestNG facilities. The former is neatly addressed by a class named AbstractTransactionalSpringContextTests, present in the Spring´s mock package. The latter, well, pretty obvious. What it isn´t so obvious is the way to combine both. Why? TestNG does not provide a direct way to surround the test cases with a transaction, so a rollback action is performed at the end of every test. This way, we are able to avoid side effects between test executions, because one test inserts a row and the next one didn´t expect it, or because a database row is updated between executions breaking some assertions, or because … you get the idea. When we decided to start using TestNG as our main framework for testing, this absence lead us to ask Cedric Beust, the TestNG project leader, about some way to accomplish with the transaction stuff. He responded us very kind and rapidly, recommending to implement the rollbacking in an @AfterMethod method.

TransactionRollbackTearDownTestCase

Note: for this class’s name origin, take a look at this site

Probably Cedric is completely right, but we like how AbstractTransactionalSpringContextTests does its business, and of course, the integration with the Spring´s IoC in order to inject the dependencies to the test classes. For that reason, we have come up with an adapter class, which permits us to run TestNG test cases within the transactional context provided by the Spring class. It is kinda adapter because we change the way the class AbstractTransactionalSpringContextTests is used (i.e. inheriting from it), right now we can extend from the new base class and define our test cases as the usual TestNG manner (i.e. through the use of annotations). It is not an adapter as in the software design patterns.

The class is named TransactionalRollbackTearDownTestCase, and following there is a diagram of its relationships:

The code of this class could prove more descriptive than me trying to explain how it does its job:

package com.centuryminds.test;

import org.springframework.test.AbstractDependencyInjectionSpringContextTests;
import org.springframework.test.AbstractTransactionalSpringContextTests;

import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeMethod;

/**
 * Base test class which permits: 1) Inject dependencies defined in a Spring
 * context. The aforementioned dependencies must be declared as protected
 * members 2) Perform a rollback action after every test. Basically, it
 * acts as an adapter between the TestNG test case structure and the facilities
 * provided by the class {@link AbstractDependencyInjectionSpringContextTests}
 * in the Spring package
 *
 * @see AbstractDependencyInjectionSpringContextTests
 * @see AfterMethod
 * @see BeforeMethod
 */
public abstract class TransactionRollbackTearDownTestCase extends
		AbstractTransactionalSpringContextTests
{

	/**
	 * Test Fixture
	 *
	 * @throws Exception if a error occurred during the fixture execution
	 * @see {@link AbstractDependencyInjectionSpringContextTests#setUp()}
	 * @see {@link AbstractDependencyInjectionSpringContextTests#setPopulateProtectedVariables(boolean)}
	 */
	@BeforeMethod
	protected final void before() throws Exception
	{
		setPopulateProtectedVariables(true);
		super.setUp();
	}

	/**
	 * Test Teardown
	 *
	 * @throws Exception if a error occurred during the teardown execution
	 * @see {@link AbstractDependencyInjectionSpringContextTests#tearDown()}
	 */
	@AfterMethod
	protected final void after() throws Exception
	{
		super.tearDown();
	}
}

As it can be seen, this class is really simple. It defines two final methods, both TestNG-annotated which delegate the test fixture and teardown actions to the underlying parent implementation. Therefore, every time a test case method is going to be executed, before() will inject the dependencies defined in the Spring context and will create and start a new transaction context. The method tearDown() in turn will perform a rollback action just after the test ends.

Extending the Hierarchy

Obviously, the class above is not enough: where are the dependencies to be injected defined? We have to tell AbstractTransactionalSpringContextTests where to look for them. This can be achieved by a subclass of TransactionalRollbackTearDownTestCase, for example:

package com.centuryminds.test;

/**
 * Base test class that specifies the spring contexts containing the dao
 * definitions. The subclasses must provide an implementation of the method
 * {@link #getConfigLocations()}, specifying the files with the spring contexts
 * holding the definitions of the dao and its dependencies
 */
public class BaseDAOTestCase extends TransactionRollbackTearDownTestCase
{
	/**
	 * @see org.springframework.test.AbstractDependencyInjectionSpringContextTests#getConfigLocations()
	 */
	@Override
	protected String[] getConfigLocations()
	{
		return new String[] { "classpath:applicationContext-datasource.xml",
				"classpath:applicationContext-service.xml" };
	}
}

The Actual Test Class

One important benefit of the separation above is that we could define several BaseDAO* classes, each one of them declaring its own context set. Let´s see an example DAO test class extending from BaseDAOTestCase:

package com.centuryminds.test;

import java.util.HashSet;

import org.testng.annotations.Test;

import com.threefish.aquarium.dao.hibernate.GenericHibernateDAO;
import com.threefish.aquarium.model.Role;
import com.threefish.aquarium.model.User;

/**
 * Tests for RoleDAO
 */
public class RoleDAOTest extends BaseDAOTestCase
{
	/** role dao */
	protected GenericHibernateDAO roleDAO;

	/** role */
	private Role role;

	@Test
	public void testCreate() throws Exception
	{
		int amount = roleDAO.findAll().size();

		// let's create a new role to persist
		role = new Role();
		role.setName("testrole");
		role.setDescription("Master of Universe");
		role.setUsers(new HashSet());

		roleDAO.makePersistent(role);

		assertNotNull(role.getId());
		assertEquals(amount + 1, roleDAO.findAll().size());
	}

	@Test
	public void testSuccessfulGet() throws Exception
	{
		role = roleDAO.findById(Long.valueOf(1));

		assertEquals("admin", role.getName());
	}

	@Test
	public void testUpdate() throws Exception
	{
		role = roleDAO.findById(Long.valueOf(1));
		role.setDescription("Super Administrator Role");
		roleDAO.makePersistent(role);

		assertEquals("Super Administrator Role", role.getDescription());
	}
}

The RoleDAO test class makes use of a custom generic DAO framework, but despite that I think it is pretty self-explanatory. We shall try to explain the generic DAO related interfaces and classes in another post when we get a chance, but for now we have enough.

The test methods are executed within a transaction per test method, which is rollbacked at the very end of it. So, the role insert in the first test (testCreate()), would be rollbacked and future tests depending on the initial state (i.e. the state before testCreate() ran) wouldn´t be affected. We accomplished with the goal of executing the test cases in isolation. Thanks Spring! We accomplished with the goal of using some of the features offered by TestNG. Thanks to you as well!

Future

We are aware about the drawbacks of this solution, and we are pretty sure about the fact that there must exist a more sophisticated solution out there, hence we are very willing to listen to your suggestions to improve the general structure of our current one. We’ll stay tuned …