» Communication in Code #1 Priority » Managed Chaos by Naresh Jain

Communication in Code #1 Priority

Here is a sample of code (happens to be from a test) which I think is not communicative.

public class ContactNumberTest {
  final String exampleCountryCode1 = "91";
  final String exampleCountryCode2 = "1";
  final String exampleNumber1 = "8012345678";
  final String exampleNumber2 = "2028569635";
 
  ContactNumber phoneNumberA = new ContactNumber(exampleCountryCode1,exampleNumber1);
  ContactNumber phoneNumberB = new ContactNumber(exampleCountryCode1,exampleNumber1);
  ContactNumber phoneNumberC = new ContactNumber(exampleCountryCode2,exampleNumber2);
 
  @Test
  public void testEquals() {
    //Symmetry
    assertTrue(phoneNumberA.equals(phoneNumberB));
    assertTrue(phoneNumberB.equals(phoneNumberA));
 
    //Reflexivity
    assertTrue(phoneNumberA.equals(phoneNumberA));
 
    //Not equals
    assertFalse(phoneNumberA.equals(phoneNumberC));
  }
 
  @Test
  public void testHashcode() {
    assertTrue(phoneNumberA.hashCode() == phoneNumberB.hashCode());
  }
}

I know you must be wondering why in the first place someone is writing tests for equals and hashCode. I agree. Its not required. But lets say someone really needs to.

This is how I would refactor the code to make it more communicative.

public class ContactNumberTest {
  private final String indiaCountryCode = "+91";
  private final String usCountryCode = "+1";
  private final String cellNumber = "8012345678";
  private final String mobileNumber = "2028569635";
 
  private final ContactNumber indianNumber = new ContactNumber(indiaCountryCode, cellNumber);
  private final ContactNumber sameIndianNumber = new ContactNumber(indiaCountryCode, cellNumber);
  private final ContactNumber usNumber = new ContactNumber(usCountryCode, mobileNumber);
  private final Map<contactnumber , String> users = new HashMap</contactnumber><contactnumber , String>();
  private String userName;
 
  @Test
  public void isSymmetrical() {
    assertNotSame(indianNumber, sameIndianNumber);
    assertEquals(indianNumber, sameIndianNumber);
    assertEquals(sameIndianNumber, indianNumber);
  }
 
  @Test
  public void isReflexive() {
    assertEquals(indianNumber, indianNumber);
  }
 
  @Test
  public void isTransitive() {
    ContactNumber newIndianNumber = new ContactNumber(indiaCountryCode, cellNumber);
    assertEquals(indianNumber, sameIndianNumber);
    assertEquals(sameIndianNumber, newIndianNumber);
    assertEquals(newIndianNumber, indianNumber);
  }
 
  @Test
  public void areNotAlwaysEqual() {
    assertFalse(indianNumber.equals(usNumber));
  }
 
  @Test
  public void equalsIsExceptionFree() {
    assertFalse(indianNumber.equals(null));
  }
 
  @Test
  public void isHashFriendly() {
    addUser("Foo").usingKey(indianNumber);
    assertEquals("Foo", users.get(indianNumber));
    addUser("Bar").usingKey(sameIndianNumber);
    assertEquals(1, users.size());
  }
 
  private void usingKey(final ContactNumber number) {
    users.put(number, userName);
  }
 
  private ContactNumberTest addUser(final String userName) {
    this.userName = userName;
    return this;
  }
}
</contactnumber>

This entry was posted on Friday, June 26th, 2009 at 2:06 PM and is filed under Agile, Design, Programming, Testing. You can follow any responses to this entry through the RSS 2.0 feed. You can skip to the end and leave a response. Pinging is currently not allowed.

http://amol-jadhav.blogspot.com/ Amol Jadhav
Nice blog. But, don’t you think the assertion
1
assertEquals(1, users.keySet().size());
would be more explicit than
1
assertEquals(1, users.size());
inside
1
@Test isHashFriendly()
?
http://amol-jadhav.blogspot.com Amol Jadhav
Nice blog. But, don’t you think the assertion
1
assertEquals(1, users.keySet().size());
would be more explicit than
1
assertEquals(1, users.size());
inside
1
@Test isHashFriendly()
?
Stephen Souness

You could also consider using assertSame in the isHashFriendly code.
Stephen Souness

You could also consider using assertSame in the isHashFriendly code.
http://ossigeno.sourceforge.net/ Giorgio Sironi

Why not testing equals()? If there is logic there (which fields should be confronted and which are ignored) it should be tested to prevent regression. And the tests also provide documentation for how equals() can be used.
http://ossigeno.sourceforge.net Giorgio Sironi

Why not testing equals()? If there is logic there (which fields should be confronted and which are ignored) it should be tested to prevent regression. And the tests also provide documentation for how equals() can be used.
http://agilefaqs.com/nareshjain.html Naresh Jain

Good suggestions @Amol and @Stephen.

@Giorgio, in my case, I use a template in my IDE to generate this code. Do you test generated code?

Also what you are referring to, is usually considered as bullet proofing and when I’m doing TDD I don’t try to bullet proof my code. I’m writing tests to drive my design not to bullet proof it.

As far a documentation goes, I would agree with you in other cases, but in the case of equals() and hashcode(), I would say all my developers should know how it works and how to use it. If they don’t, then we have a bigger issue to deal with.
http://agilefaqs.com/nareshjain.html Naresh Jain

Good suggestions @Amol and @Stephen.

@Giorgio, in my case, I use a template in my IDE to generate this code. Do you test generated code?

Also what you are referring to, is usually considered as bullet proofing and when I’m doing TDD I don’t try to bullet proof my code. I’m writing tests to drive my design not to bullet proof it.

As far a documentation goes, I would agree with you in other cases, but in the case of equals() and hashcode(), I would say all my developers should know how it works and how to use it. If they don’t, then we have a bigger issue to deal with.