Search code examples
javaclasstreemap

Using a TreeMap as a property in a Person object class


Is it wrong practice to write my code as following? I want to be able to store an email in my person class that also contains the type of email it is (Work, Personal, etc). I decided to use a TreeMap for this. I know that it is good practice for all of your variables to be private and to use getter and setters to manipulate them, but is it wrong to manipulate my TreeSet by directly using the TreeSet methods, rather than my own in my Person class? In other words, is this a valid acceptable way of doing this? The code seems to work fine.

public class Person {
  private String firstName;
  private String lastName;
  private String note;
  TreeMap<String, String> phoneNum = new TreeMap<String, String>();

  // Assume constructor method contains firstName & lastName and there are
  // getters and setters for both
}

public class MainDriver {
  public static void main(String[] args) {
    Person p1 = new Person("John", "Smith");

    p1.phoneNum.put("jsmith@gmail.com", "School");
    p1.phoneNum.put("jsmith19@gmail.com", "Personal");

    Person p2 = new Person("Sam", "Johnson");

    p2.phoneNum.put("samjohn@gmail.com", "Personal");
    p2.phoneNum.put("samjohnson", "Work");

    System.out.println(p1.phoneNum);
    System.out.println(p2.phoneNum);
  }
}

Output:
{jbillingham@gmail.com=Personal, jebillingham3@gmail.com=School}
{samsamyoussef=Work, samyou@gmail.com=Personal}

Solution

  • It's not terrible, but it enables feature envy (the code smell whereby objects use the fields of other objects directly).

    The problem is that you wish only to enable adding email addresses to the Person, but you actually expose all of the operations of the TreeMap. Methods like ceilingKey, tailMap and remove. In order to limit the operations that can be performed you should fully encapsulate the field and provide explicit methods.