Designing classes and methods for hospital management system

I am working on hospital management system project. I have implemented it using the following design. Is it a good design?

I have created four classes: Patient, Doctor, Hospital, and Demo. Inside the Hospital class are two ArrayLists of Patients and Doctors. Also, inside the Doctor class, there is a list of Patients (this is a specific Patient list of specific Doctor).

Inside the Demo class, I am adding Patients and Doctors, and they are being added to respective lists of the Hospital class.

Demo

```
public class demo {

public static void main (String args[])

{
Hospital h1= new Hospital("Appollo");
int choice=0;
Doctor o = new Doctor("Rai","Surgeon");
Doctor o1 = new Doctor("James","Opthalmologist");
Doctor o2 = new Doctor("Steve","ENT");
System.out.println("Press 1 to add doctor \n Press 2 to show doctors.\n Press 3 to add patient \n 4 Assign doctor to patients\n 5 Doctor and their patients ");
Scanner cin = new Scanner (System.in);
choice = cin.nextInt();
switch (choice)
{
case 1: h1.addDoctor(o);
h1.addDoctor(o1);
h1.addDoctor(o2);
// break;
case 2: { System.out.println(h1.showDoctors());

}
case 3: { Patient p = new Patient ("Steven ",21,"Male","eye");
Patient p1 = new Patient ("Michael ",12,"Male","heart patient");
Patient p2 = new Patient ("Sara ",23,"Female","earnose");
Patient p3 = new Patient ("Amy ",31,"Female","earnose");
Patient p5 = new Patient ("Rocky ",18,"Male","eye");
Patient p4= new Patient ("Jessy ",15,"Male","heart patient");
h1.addPatient(p);
h1.addPatient(p1);
h1.addPatient(p2);
h1.addPatient(p3);
h1.addPatient(p4);
h1.addPatient(p5);

-
Indentation: It's all over the place and kind of hard to follow. Try to be consistent.

-
Braces: Same thing. For the most part you are putting them at the beginning of the next line. But sometime you are putting a line of code with the brace. This is not one of the standard conventions. Using a standard convention is better than using a non-standard one, but in either case, you should be consistent.

-
Switch Braces: This marks a scoped region. It does not mean you don't need break statements.

-
Variable Names: Pick good variable names:

In assignDoctor()

for (Patient x: patientList) {
  for (Doctor y: doctorList) {
    // ...
  }
}

x and y are arbitrary and mean nothing. As you get father down the loop, names like patient and doctor are going to be helpful.

In Hospital, doctorList and patientList can just be doctors and patients. The type system will tell you the variable is a list.

In Doctor, everything is prefixed with "doctor". It's a field of a Doctor, so it is understood that the field belongs to a doctor. Same with Patient.

• Variables in General: If you are only going to use a variable for one thing, you don't need to make one.

Ex:

Patient p = new Patient ("Steven ",21,"Male","eye");
Patient p1 = new Patient ("Michael ",12,"Male","heart patient");
Patient p2 = new Patient ("Sara ",23,"Female","earnose");
Patient p3 = new Patient ("Amy ",31,"Female","earnose");
Patient p5 = new Patient ("Rocky ",18,"Male","eye");
Patient p4= new Patient ("Jessy ",15,"Male","heart patient");
h1.addPatient(p);
h1.addPatient(p1);
h1.addPatient(p2);
h1.addPatient(p3);
h1.addPatient(p4);
h1.addPatient(p5);

can just be:

h1.addPatient(new Patient ("Steven ",21,"Male","eye"));
h1.addPatient(new Patient ("Michael ",12,"Male","heart patient"));
// ...

-
Values: Not everything is a String. The type system is there to help you. But when you make something like sex a String, there is nothing stopping you form treating "dslkjahgkfdj" as a gender.

The same goes for patient disease and doctor specialty. You have a fixed set of values you are expecting, yet you are leaving the door open for many others. What should happen in assignDoctor() if a patient has "school bus" and the only doctor specializes in "envelope"? An enum would likely be a good match for this type of thing. It allows you to define a set of values and know you will always be dealing with something in that set.

If you do decide to stick with String, you need to define the expected values as constants. If tomorrow you want to change "heart patient" to "cardiac", it will be much easier to change the value of a single static final variable then find all of the instances of "heart patient" in the code base.

StackExchange Code Review Q#29965, answer score: 8