Find upcoming annual and quarterly interviews

I am writing a management application to replace an existing application. Depending on certain criteria, people are interviewed either quarterly or annually based on certain criteria. The date of the interviews are determined by their birthday. I want a dialog that will display all upcoming interviews, one month in advance for annual interviews and one week in advance for quarterly interviews.

I'd like to ask for a general review, especially since this part started as a "hey, can this be put into the program real quick-like?" kind of request and turned into Frankencode by the time I was done.

The applicable code is showUpcomingForm() (called from the main app), SORDate.java (customized calendar object), and UpcomingDialog.java. The code for UpcomingDialog isn't shown--it's pretty basic, just a dialog with a table to display results.

showUpComingForm():

```
private static void showUpcomingForm() {
SimpleDateFormat sdf = new SimpleDateFormat("MM/dd/yyyy");
List upcomingList = new ArrayList<>();

//go thru entire interviewee list
Iterator i = intervieweeList.iterator();
while (i.hasNext()) {
try {
Interviewee o = (Interviewee) i.next();

//Interviewee has an unsorted hashset of Interview objects containing 0-n
//Interviews. Gets the last interview using InterviewComparator
//(compares database id of interview object). If there are no objects
//in the set, create a blank object to preventnull pointer error.
Interview interview = null;
if(o.getInterviews().size()>0){
//get latest interview
interview = (Interview) Collections.max(o.getInterviews(),
new InterviewComparator());
}else{
interview = new Interview();
}
//create calendar objects
SORDate nowDate = new SORDate(new Date());

//if there is not date of bir

At first glance,

• o is a terrible variable name

• getIsActive boolean properties typically don't use get. I know some source code generators will throw it on there and that is probably what happened in this case.

Encapsulation:

Validation

if(o.getDateOfBirth()==null ||
   o.getDateOfBirth().equals(""))

if (interview.getInterviewDate() != null && 
   !interview.getInterviewDate().equals(""))

Move it into the respective classes

• if(individual.hasDateOfBirth())

• if(interview.hasDate())

Similarly, interview intialization would read much better as:

Interview interview = individual.hasInterview() ? individual.lastInterview() : new Interview();

The = null goes away in addition to allowing the lastInterview method to assume the responsibility of determining the "last" interview.

Maintainability

o.getRegistrationObligation().equals("ANNUAL") and o.getRegistrationObligation().equals("90 DAYS") sway me toward declaring an Enum. As the usages of .equals("Some String") propagate, so to do the number of places you would have to fix it if the value were to ever change. Enum encapsulates that value and provides a single point of value remediation (to a certain degree).

String Building

Beyond a couple + concatenation usages, I like to switch to String.format.

String.format("%s, %s has no date of birth listed.\nDPS Number: %s",       
               o.getLastName(),  
               o.getFirstName() 
               o.getDpsNumber()) //assuming this is a String

This would also allow for placement of the resource (the String) into a ResourceBundle if that particular error message were to ever need to be reused elsewhere. In addition, the usage of a bundle encapsulates all non-variable based String resources to one place.

StackExchange Code Review Q#52101, answer score: 5