Collection of Collections Is a Code Smell

From WikiContent

(Difference between revisions)
Jump to: navigation, search
Line 1: Line 1:
-
Have you run into, or coded yourself into a place where the obvious solution was to create a HashMap of HashMaps? How about a HashMap of ArrayList or come other combination of collection of collections. While some things, such a matrices, are naturally represented this way, more often than not, creating a collection of collections is an indication of a missing design element. It is because of tendency that the coding pattern would be considered a "code smell".
+
Have you run into, or coded yourself into a place, where the obvious solution was to create a <code>HashMap</code> of <code>HashMaps</code>? How about a <code>HashMap</code> of <code>ArrayList</code> or any other combination of a collection of collections. While some things, such a matrices, are naturally represented this way, more often than not creating a collection of collections is an indication of a missing design element. It is because of tendency that the coding pattern would be considered a ''code smell''.
-
The term "code smell" was first coined by Kent Beck. It is used to describe code that is awkward looking or doesn't look right. It is the awkwardness that tends to point to some deeper underlying problem. In the example above, the code smell is often an indication that one needs an object to more naturally represent a relationship between the keys of the first outer collection and the values of the inner collections. This is perhaps best seen in a small example such as that provided in listing 1.
+
The term ''code smell'' was first coined by Kent Beck. It is used to describe code that is awkward looking or doesn't look right. It is the awkwardness that tends to point to some deeper underlying problem. In the example above, the code smell is often an indication that one needs an object to more naturally represent a relationship between the keys of the first outer collection and the values of the inner collections. This is perhaps best seen in a small example such as that provided in listing 1.
-
public class AllPersons {
+
public class AllPersons {
 +
 +
private HashMap<String,HashMap> allPersons = new HashMap<String,HashMap>();
 +
 +
public void addPerson(Person person) {
 +
HashMap<String,HashMap> persons = this.allPersons.get(person.getLastName());
 +
if (persons == null) {
 +
persons = new HashMap<String,Person>();
 +
this.allPersons.add(person.getLastName(), persons);
 +
}
 +
persons.add(person.getFirstName(), person);
 +
}
 +
}
-
private HashMap<String,HashMap> allPersons = new HashMap<String,HashMap>;
+
Listing 1. <code>AllPersons</code>, an implied collection.
-
public void addPerson( Person person) {
+
This class <code>AllPersons</code> is an implied collection that is keyed on a persons last name. The inner collection is then keyed on the persons first name. The code smell in the example is that we are keying on the last name and then the first. The question is, what is the missing design element if there is one.
-
HashMap<String,HashMap> persons = this.allPersons.get( person.getLastName());
+
-
if ( persons == null) {
+
-
persons = new HashMap<String,Person>();
+
-
this.allPersons.add( person.getLastName(), persons);
+
-
}
+
-
persons.add( person.getFirstName(), person);
+
-
}
+
-
}
+
-
Listing 1. AllPersons, an implied collection.
+
When we use a <code>HashMap</code> we are implicitly creating an index much in the same way we'd create an index in a database table. If we were to create an index on a single column, that would be a simple key. If we combine two or more simple keys to create another index, we have created a compound key. And this is exactly what we are doing in this example, creating an index based on two fields. From this we can conclude that the missing design element is a compound key. Listing 2. demonstrates the code with our newly discovered class.
-
This class AllPersons is an implied collection that is keyed on a persons last name. The inner collection is then keyed on the persons first name. The code smell in the example is that we are keying on the last name and then the first. The question is, what is the missing design element if there is one.
+
public class AllPersons {
 +
 +
private HashMap<CompoundKey,Person> allPersons = new HashMap<CompoundKey,Person>();
 +
 +
public void addPerson(Person person) {
 +
this.addPerson(new CompoundKey(person.getFirstName(), person.getLastName(), person);
 +
}
 +
}
-
When we use a HashMap we are implicitly creating an index much in the same way we'd create an index in a database table. If we were to create an index on a single column, that would be a simple key. If we combine two or more simple keys to create another index, we have created a compound key. And this is exactly what we are doing in this example, creating an index based on two fields. From this we can conclude that the missing design element is a compound key. Listing 2. demonstrates the code with our newly discovered class.
+
Listing 2. <code>AllPersons</code>, an implied collection using a <code>CompoundKey</code>.
-
public class AllPersons {
+
Although it is perhaps a bit difficult to see in this short example, adding a needed design element often results in far less code. Not only will we have less code to read, the code we are left with is more readable. In this case, the code also has a better performance profile. All of these benefits came when as a result of paying attention to and acting on code smells.
-
private HashMap<CompoundKey,Person> allPersons = new HashMap<CompoundKey,Person>;
+
By [[Kirk Pepperdine]]
-
public void addPerson( Person person) {
+
This work is licensed under a [http://creativecommons.org/licenses/by/3.0/us/ Creative Commons Attribution 3]
-
this.addPerson( new CompoundKey( person.getFirstName(), person.getLastName(), person);
+
-
}
+
-
}
+
-
Listing 2. AllPersons, an implied collection using a CompoundKey.
 
-
Though it is perhaps a bit difficult to see in this short example, adding a needed design element often results in far less code. Not only will we have less code to read, the code we are left with is more readable. In this case, the code also has a better performance profile. All of these benefits came when as a result of paying attention to and acting on code smells.
+
 
 +
Back to [[97 Things Every Programmer Should Know]] home page

Revision as of 00:35, 18 December 2008

Have you run into, or coded yourself into a place, where the obvious solution was to create a HashMap of HashMaps? How about a HashMap of ArrayList or any other combination of a collection of collections. While some things, such a matrices, are naturally represented this way, more often than not creating a collection of collections is an indication of a missing design element. It is because of tendency that the coding pattern would be considered a code smell.

The term code smell was first coined by Kent Beck. It is used to describe code that is awkward looking or doesn't look right. It is the awkwardness that tends to point to some deeper underlying problem. In the example above, the code smell is often an indication that one needs an object to more naturally represent a relationship between the keys of the first outer collection and the values of the inner collections. This is perhaps best seen in a small example such as that provided in listing 1.

public class AllPersons {

    private HashMap<String,HashMap> allPersons = new HashMap<String,HashMap>();

    public void addPerson(Person person) {
        HashMap<String,HashMap> persons = this.allPersons.get(person.getLastName());
        if (persons == null) {
            persons = new HashMap<String,Person>();
            this.allPersons.add(person.getLastName(), persons);
        }
        persons.add(person.getFirstName(), person);
    }
}

Listing 1. AllPersons, an implied collection.

This class AllPersons is an implied collection that is keyed on a persons last name. The inner collection is then keyed on the persons first name. The code smell in the example is that we are keying on the last name and then the first. The question is, what is the missing design element if there is one.

When we use a HashMap we are implicitly creating an index much in the same way we'd create an index in a database table. If we were to create an index on a single column, that would be a simple key. If we combine two or more simple keys to create another index, we have created a compound key. And this is exactly what we are doing in this example, creating an index based on two fields. From this we can conclude that the missing design element is a compound key. Listing 2. demonstrates the code with our newly discovered class.

public class AllPersons {

    private HashMap<CompoundKey,Person> allPersons = new HashMap<CompoundKey,Person>();

    public void addPerson(Person person) {
        this.addPerson(new CompoundKey(person.getFirstName(), person.getLastName(), person);
    }
}

Listing 2. AllPersons, an implied collection using a CompoundKey.

Although it is perhaps a bit difficult to see in this short example, adding a needed design element often results in far less code. Not only will we have less code to read, the code we are left with is more readable. In this case, the code also has a better performance profile. All of these benefits came when as a result of paying attention to and acting on code smells.

By Kirk Pepperdine

This work is licensed under a Creative Commons Attribution 3


Back to 97 Things Every Programmer Should Know home page

Personal tools