My first attempt at doing some OOP that isn't simple practice exercises is to make a program in java which stores information about people (like a contact book). Below is a snippet of a class called Person which I'm working on. Person will be instantiated by some other class some time in the future.
public class Person {
// Enum of the fields in Person
private static enum Field {NAME, ALIASES, DATE_OF_BIRTH, DATE_OF_DEATH, VITAL_STATUS,
RELATIONSHIPS}
private static enum VitalStatus {ALIVE, DECEASED, ASSUMED_DECEASED, UNKNOWN}
private final int id;
private Name name;
private List<String> aliases;
private GregorianCalendar dateOfBirth;
private GregorianCalendar dateOfDeath;
private VitalStatus vitalStatus;
private List<Relationship> relationships;
// Time field was last updated
private Map<Field, GregorianCalendar> updateTimes;
// Initialise a blank/unknown person
public Person() {
this.id = getNewId();
this.name = new Name(); // Blank name
this.aliases = new ArrayList<String>();
this.dateOfBirth = null;
this.dateOfDeath = null;
this.vitalStatus = VitalStatus.UNKNOWN;
this.relationships = new ArrayList<Relationship>();
// Initialise all update times to null
this.updateTimes = new HashMap<Field, GregorianCalendar>();
for(Field f : Field.values()) {
updateTimes.put(f, null);
}
}
// Update time methods
private void updateUpdateTime(Field field) {
updateTimes.put(field, new GregorianCalendar());
}
public GregorianCalendar getNameUpdateTime() {
return updateTimes.get(Field.NAME);
}
public GregorianCalendar getAliasesUpdateTime() {
return updateTimes.get(Field.ALIASES);
}
public GregorianCalendar getDateOfBirthUpdateTime() {
return updateTimes.get(Field.DATE_OF_BIRTH);
}
public GregorianCalendar getDateOfDeathUpdateTime() {
return updateTimes.get(Field.DATE_OF_DEATH);
}
public GregorianCalendar getVitalStatusUpdateTime() {
return updateTimes.get(Field.VITAL_STATUS);
}
public GregorianCalendar getRelationshipsUpdateTime() {
return updateTimes.get(Field.RELATIONSHIPS);
}
// Code removed from here down
}
I'm thinking of replacing the getters for the update times with the code shown below.
public getUpdateTime(Field field) {
return updateTimes.get(field);
}
Should I make the replacement? I'm hesitant to make a getter with an argument, but it would reduce the number of lines of code and would allow me to add new fields and update times without needing to write new getters.
You might want to add that as an option in this case, but I would recommend against it in general. Having used both the java.util.Calendar
API (where you specify the field number you want to fetch or update) and the APIs of Joda Time and java.time
, where you can just say getHour()
, getMonth()
etc, the latter is much simpler to use.
Ignore how many lines of code it takes you to add new fields - concentrate on how easy it is to use the API you're providing. Will callers ever need to generalize over fields? If so, having that extra method makes sense. If not - if callers always know exactly which of those values they want at the call site, at compile-time - then I'd stick with the original form.
I do understand that your situation is a little different to the calendar one in that you've got the same "aspect" (last update time) for multiple fields, but I'm still cautious about what it would be like from the caller's perspective.
(I have no problem with having a parameter in a getter in general. Do whatever leads to the most useful API.)
As an aside, I would steer away from GregorianCalendar
if you possibly can - do you really want callers to be able to change the last update time? They can at the moment, because GregorianCalendar
is mutable. Switch to Joda Time or java.time
if you possibly can.
See more on this question at Stackoverflow