Designing classes and methods for hospital management system

Question

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);
                     System.out.println(h1.showPatients());
                }   
    case 4:     {
                     h1.assignDoctor();
                }

    case 5:     {
                      System.out.println("\n \n \n "+ ""+o2.getDoctorName()+""+o2.getDoctorPatientList());
                }
    }
    }}

Hospital

public class Hospital {

List <Doctor> doctorList = new ArrayList<Doctor>();
List <Patient> patientList = new ArrayList<Patient>();
String hospitalName;
void addDoctor(Doctor o)
{
    doctorList.add(o);

}
void addPatient(Patient o)
{
    patientList.add(o);
}
Hospital(String name)
{
    this.hospitalName=name;
}

public List<Doctor> showDoctors()
{
    return doctorList;
}
public List<Patient> showPatients()
{
    return patientList;
}

public void assignDoctor()
{
    for (Patient x: patientList)
    {      for (Doctor y: doctorList)
            {     if (x.getDisease().equals("eye"))
                        {
                            if (y.getDoctorspeciality().equals("Opthalmologist"))
                                {
                                   y.addPatientsToDoctor(x);
                                }
                        }
            if (x.getDisease().equals("heart patient"))
            {
                if (y.getDoctorspeciality().equals("Surgeon"))
                    {
                       y.addPatientsToDoctor(x);
                    }
            }

            if (x.getDisease().equals("earnose"))
            {
                if (y.getDoctorspeciality().equals("ENT"))
                    {
                       y.addPatientsToDoctor(x);
                    }
            }

            }
        }

}


}

Doctor

public class Doctor {

private String  doctorName;
private String  doctorSpeciality;
String  doctorStatus;
 List<Patient> doctorPatientList= new ArrayList<Patient>();
Doctor(String c, String cc)
{
    this.doctorName=c;
    this.doctorSpeciality=cc;

}
public String  getDoctorName()
{
    return doctorName;
}

public List<Patient> getDoctorPatientList()
{   
    return doctorPatientList;
}

public void addPatientsToDoctor(Patient o)
{
    doctorPatientList.add(o);
}

String getDoctorspeciality()
{
    return doctorSpeciality;
}
public String toString()
{
    return (doctorName+""+doctorSpeciality);
}

}

Patient

public class Patient {
    private String patientName;
    private int patientAge;
    private String  patientGender;
    private String disease;
    Patient (String patientName, int patientAge,String patientGender, String disease)
    {
        this.patientName= patientName;
        this.patientGender= patientGender;
        this.patientAge=patientAge;
        this.disease=disease;
    }   

    public String getDisease()
    {return disease;}

    public String toString()
    {
        return (patientName+""+patientAge+""+patientGender +" "+ disease);
    }
}

Answer 1

• 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.

Answer 2

Building upon @unholysampler's excellent feedback, if the matching condition between a disease and speciality is as straightforward as your current example, and the hospital will also only have one doctor per speciality, you can make use of two Maps in assignPatientToDoctor (renamed) to make the assignment simpler.

I think it's realistic to make these two assumptions given the simplistic requirements. Besides, your code seems to suggest that one patient can be added to multiple doctors. Is that desired? Even if the assumptions are not true, you can tweak it slightly to have the value as a List<Doctor> for example, and then build some logic to pick out which doctor do you want to add the patient with the disease to. Also, in my code below, I did not check if a Patient is added multiple times to a Doctor's list, maybe you want to implement that as well.

Let's start off with @unholysampler's recommendation for enums first, I am providing Doctor and Patient as an interface here only to illustrate that and the recommended change in method names:

The Doctor interface:

package com.myproject;

import java.util.List;

public interface Doctor {

    public enum Speciality {
        OPTHALMOLOGIST, SURGEON, ENT
    }

    public String getName();

    public Speciality getSpeciality();

    public List<Patient> getPatients();

    public void addPatients(Patient... patients);
}

One significant change here: addPatients accept variable arguments ("varargs") as the name implies one can add one or more patients (of course, null/empty checks should be done here).

The Patient interface:

package com.myproject;


public interface Patient {

    public enum Disease {
        EYE, HEART_PATIENT, EARNOSE
    }

    public Disease getDisease();

}

The Hospital class, specifically the tweaked assignPatientToDoctor method:

package com.myproject;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import com.myproject.Doctor.Speciality;
import com.myproject.Patient.Disease;

public class Hospital {

    static Map<Patient.Disease, Doctor.Speciality> specialityHandlingForDisease = new HashMap<Patient.Disease, Doctor.Speciality>();
    Map<Doctor.Speciality, Doctor> doctors = new HashMap<Doctor.Speciality, Doctor>();
    List<Patient> patients = new ArrayList<Patient>();

    static {
        specialityHandlingForDisease.put(Disease.EYE, Speciality.OPTHALMOLOGIST);
        specialityHandlingForDisease.put(Disease.HEART_PATIENT, Speciality.SURGEON);
        specialityHandlingForDisease.put(Disease.EARNOSE, Speciality.ENT);
    }

    public void addDoctor(Doctor doctor) {
        doctors.put(doctor.getSpeciality(), doctor);
    }

    public void assignPatientToDoctor() {
        for (Patient patient : patients) {
            if (specialityHandlingForDisease.containsKey(patient.getDisease())) {
                Speciality speciality = specialityHandlingForDisease.get(patient.getDisease());
                if (doctors.containsKey(speciality)) {
                    doctors.get(speciality).addPatients(patient);
                }
            }
        }
    }
}