Return to Answer

• 1

• 19.3k
• 3
• 37
• 93

Bracing style

You started off with Allman-style bracing approach, but switched to the Java convention for HelloWorld and its main() method. Regardless of the styles you choose, please be consistent on this front. :)

Medical condition dates

Your medical conditions carry a start date, but should they also have an end date? (削除) Or are we only talking about incurable medical conditions here? (削除ここまで) While we're at it, Java 8 has new 'Time' APIs with a different (and better) approach to modeling chronology, so you may want to use the newer LocalDate class.

OK, now I get it... you delete medical conditions when they are cured. If you need to preserve medical history, you now know what you can do too...

Throwing Exceptions

public static void main(String args[]) throws ParseException

I understand HelloWorld is something like a test class to see your implementation in action, but still try to avoid doing something like this, especially when it's your main() method. One wrong input will simply cause your test class to quit with a stacktrace. It will be better if you have a method to handle the parsing and any possible errors. For example, if you want to default to the current date in case of parsing errors:

private static LocalDate parse(String date) {
 // using the new time APIs
 try {
 return DateTimeFormatter.ISO_LOCAL_DATE.parse(date, LocalDate::from);
 } catch (DateTimeParseException e) {
 return LocalDate.now();
 }
}

equals() implementation

Do I need to implement an equals method in the MedicalCondition class?

Your call. :) The longer answer is, what does it mean when a medical condition is-equal to another? Same condition and date? How helpful will such an equivalence be? To 'group' patients by the same medical conditions? If you don't have a strong case for this, you probably wouldn't want to implement it first.

Adding medical conditions

In this regard, I will recommend your existing approach over @Amr Ayman's answer. This is mainly because your Patient class should know what it means to add a medical condition, instead of letting callers of your Patient instances 'figure that out'. A better refinement is to wrap the instance returned by getMedicalConditions() with a Collections.unmodifiableList(): this will prevent callers of the method to modify the list's contents directly, such as wiping them outright.

Actually, on this note, you can also consider making your MedicalCondition class immutable, and I shall leave that as an exercise for the healthy reader...