Object-oriented Quarantine implementation

Question

I am a newbie in thinking object oriented solution for a problem, so I am trying to improve. I have tried to implement a quarantine assignment.

Here is a requirement with unit test cases. I am trying to make this assignment scalable as much as possible with a good object oriented design.

public class QuarantineTest {

    private Quarantine quarantine;

    @Before
    public void setUp() {
        // The responsibility of the Quarantine object is to simulate diseases on a group of patients.
        // It is initialized with a list of patients' health status, separated by a comma.
        // Each health status is described by one or more characters
        // (in the test below, we will always have only one disease / patient)
        // The characters mean:
        // H : Healthy
        // F : Fever
        // D : Diabetes
        // T : Tuberculosis
        quarantine = new Quarantine("F,H,D,D,D,H,T");

        // Quarantine provides medicines to the patients, but can not target a specific group of patient.
        // The same medicines are always given to all the patients.

        // Then Quarantine can provide a report with this format:
        // "F:1 H:2 D:0 T:1 X:3"
        // Report give the number of patients that have the given disease.
        // X means Dead
    }

    @Test
    public void beforeTreatment() throws Exception {
        assertEquals("F:1 H:2 D:3 T:1 X:0", quarantine.report());
    }

    // people died in the Diabetes
    @Test
    public void noTreatment() throws Exception {
        quarantine.wait40Days();
        // diabetics die without insulin
        assertEquals("F:1 H:2 D:0 T:1 X:3", quarantine.report());
    }

    // feaver is cured
    //  people died in the Diabetes
    @Test
    public void aspirin() throws Exception {
        quarantine.aspirin();
        quarantine.wait40Days();
        // aspirin cure Fever
        assertEquals("F:0 H:3 D:0 T:1 X:3", quarantine.report());
    }

    @Test
    public void antibiotic() throws Exception {
        quarantine.antibiotic();
        quarantine.wait40Days();
        // antibiotic cure Tuberculosis
        // but healthy people catch Fever if mixed with insulin.
        assertEquals("F:1 H:3 D:0 T:0 X:3", quarantine.report());
    }

    @Test
    public void insulin() throws Exception {
        quarantine.insulin();
        quarantine.wait40Days();
        // insulin prevent diabetic subject from dying, does not cure Diabetes,
        assertEquals("F:1 H:2 D:3 T:1 X:0", quarantine.report());
    }

    @Test
    public void antibioticPlusInsulin() throws Exception {
        quarantine.antibiotic();
        quarantine.insulin();
        quarantine.wait40Days();
        // if insulin is mixed with antibiotic, healthy people catch Fever
        assertEquals("F:3 H:1 D:3 T:0 X:0", quarantine.report());
    }

    @Test
    public void paracetamol() throws Exception {
        quarantine.paracetamol();
        quarantine.wait40Days();
        // paracetamol heals fever
        assertEquals("F:0 H:3 D:0 T:1 X:3", quarantine.report());
    }

    @Test
    public void paracetamolAndAspirin() throws Exception {
        quarantine.paracetamol();
        quarantine.aspirin();
        quarantine.wait40Days();
        // paracetamol kills subject if mixed with aspirin
        assertEquals("F:0 H:0 D:0 T:0 X:7", quarantine.report());
    }

}

This is what I have implemented:

package quarantine;

import java.util.ArrayList;
import java.util.LinkedHashMap;

public class Quarantine {

    LinkedHashMap<PatientType,Integer> patientInfoMap =  new LinkedHashMap<>();
    private  ArrayList<Medicines> appliedTreatementList = new ArrayList<>();
     ArrayList<ITreatment> treatementRepository = new ArrayList<>();
    static int treatedTBPatientCount=0;
    static boolean isInsulineInjected = false;

    public Quarantine(String subjects) {
        String[] patientArr = subjects.split(",");
        for(String disease :patientArr){
            int patientCount = patientInfoMap.get(PatientType.getType(disease)) != null ? patientInfoMap.get(PatientType.getType(disease)):0;
            patientInfoMap.put(PatientType.getType(disease),++patientCount);
        }
        patientInfoMap.put(PatientType.DEAD,0);
    }

    public String report() {
        try {
            final String[] result = {""};
            patientInfoMap.forEach((k, v) -> result[0] += PatientType.getValue(k) + ":" + v.toString() + " ");
            return result[0].trim();
        } catch (Exception e) {
            e.printStackTrace();
        }
        return null;
    }

    public void wait40Days() {
        for(ITreatment treatment : treatementRepository){
            treatment.treat(patientInfoMap,appliedTreatementList);
        }
        if(!isInsulineInjected){
            dieDiabticPatient();
        }
    }

    private void dieDiabticPatient() {
        patientInfoMap.put(PatientType.DEAD, patientInfoMap.get(PatientType.DIABATIC));
        patientInfoMap.put(PatientType.DIABATIC, 0);

    }

    public void aspirin() {
        appliedTreatementList.add(Medicines.ASPIRIN);
        treatementRepository.add(new TreatmentAspirin());

    }

    public void antibiotic() {
        appliedTreatementList.add(Medicines.ANTIBIOTIC);
        treatementRepository.add(new TreatmentAntibiotic());

    }

    public void insulin() {
        appliedTreatementList.add(Medicines.INSULIN);
        treatementRepository.add(new TreatmentInsulin());

    }

    public void paracetamol() {
        appliedTreatementList.add(Medicines.PARACETOL);
        treatementRepository.add(new TreatmentParacetamol());

    }

}

TreatmentAntibiotic class:

package quarantine;

import java.util.ArrayList;
import java.util.LinkedHashMap;

public class TreatmentAntibiotic implements ITreatment{

    @Override
    public void treat(LinkedHashMap<PatientType, Integer> patientInfoMap, ArrayList<Medicines> appliedTreatementList) {
        if(appliedTreatementList.contains(Medicines.ANTIBIOTIC)){
            Quarantine.isInsulineInjected = false;
            LinkedHashMap<PatientType, Integer> map = patientInfoMap;
            Quarantine.treatedTBPatientCount = map.get(PatientType.TUBERCLOSIS);
            map.put(PatientType.HEALTHY, map.get(PatientType.HEALTHY) + map.get(PatientType.TUBERCLOSIS));
            map.put(PatientType.TUBERCLOSIS, 0);
        }



    }

}

ITreatment interface:

package quarantine;

import java.util.ArrayList;
import java.util.LinkedHashMap;

interface ITreatment {


    abstract public  void treat(LinkedHashMap<PatientType, Integer> patientInfoMap, ArrayList<Medicines> appliedTreatementList);
}

enum class:

    package quarantine;

enum Medicines {
    INSULIN,
    ANTIBIOTIC,
    PARACETOL,
    ASPIRIN
}

PatientType class:

    package quarantine;

public enum PatientType {
    DEAD("X"),
    DIABATIC("D"),
    FEVER("F"),
    TUBERCLOSIS("T"),
    HEALTHY("H");
    private final String disease;

    PatientType(String disease) {
            this.disease = disease;
    }
    public static String getValue(PatientType patientType)
    {
        for (PatientType type : PatientType.values())
            if (type.equals(patientType))
                return type.disease;
        return null;
    }
    public static PatientType getType(String diseaseType)
    {
        for (PatientType type : PatientType.values())
            if (type.disease.equals(diseaseType))
                return type;
        return null;
    }


}

TreatmentAntibiotic class:

package quarantine;

import java.util.ArrayList;
import java.util.LinkedHashMap;

public class TreatmentAntibiotic implements ITreatment{

    @Override
    public void treat(LinkedHashMap<PatientType, Integer> patientInfoMap, ArrayList<Medicines> appliedTreatementList) {
        if(appliedTreatementList.contains(Medicines.ANTIBIOTIC)){
            Quarantine.isInsulineInjected = false;
            LinkedHashMap<PatientType, Integer> map = patientInfoMap;
            Quarantine.treatedTBPatientCount = map.get(PatientType.TUBERCLOSIS);
            map.put(PatientType.HEALTHY, map.get(PatientType.HEALTHY) + map.get(PatientType.TUBERCLOSIS));
            map.put(PatientType.TUBERCLOSIS, 0);
        }



    }

}

TreatmentAspirin class:

    package quarantine;

import java.util.ArrayList;
import java.util.LinkedHashMap;

public class TreatmentAspirin implements ITreatment{


    @Override
    public void treat(LinkedHashMap<PatientType, Integer> patientInfoMap, ArrayList<Medicines> appliedTreatementList) {

        if(appliedTreatementList.contains(Medicines.ASPIRIN)){
            LinkedHashMap<PatientType, Integer> map = patientInfoMap;

            if(appliedTreatementList.contains(Medicines.PARACETOL)){
                Quarantine.isInsulineInjected = true; //Because They are Dead
                map.put(PatientType.DEAD, map.get(PatientType.DEAD) + map.get(PatientType.FEVER) + map.get(PatientType.HEALTHY) + map.get(PatientType.DIABATIC) + map.get(PatientType.TUBERCLOSIS));
                map.put(PatientType.HEALTHY, 0);
                map.put(PatientType.DIABATIC, 0);
                map.put(PatientType.TUBERCLOSIS, 0);
            }else{
                Quarantine.isInsulineInjected = false;
                map.put(PatientType.HEALTHY, map.get(PatientType.HEALTHY) + map.get(PatientType.FEVER));
                map.put(PatientType.FEVER, 0);
            }

        }

    }

}

TreatmentInsulin class:

package quarantine;

import java.util.ArrayList;
import java.util.LinkedHashMap;

public class TreatmentInsulin implements ITreatment{

    @Override
    public void treat(LinkedHashMap<PatientType, Integer> patientInfoMap, ArrayList<Medicines> appliedTreatementList) {
        if(appliedTreatementList.contains(Medicines.INSULIN)){
            LinkedHashMap<PatientType, Integer> map = patientInfoMap;
            Quarantine.isInsulineInjected = true;
            if(appliedTreatementList.contains(Medicines.ANTIBIOTIC)){
                map.put(PatientType.FEVER, map.get(PatientType.HEALTHY) + map.get(PatientType.FEVER)-Quarantine.treatedTBPatientCount);
                map.put(PatientType.HEALTHY, Quarantine.treatedTBPatientCount);

            }
        }

    }

}

TreatmentParacetamol class:

package quarantine;

import java.util.ArrayList;
import java.util.LinkedHashMap;

public class TreatmentParacetamol implements ITreatment{

    @Override
    public void treat(LinkedHashMap<PatientType, Integer> patientInfoMap, ArrayList<Medicines> appliedTreatementList) {
        if(appliedTreatementList.contains(Medicines.PARACETOL)){
            Quarantine.isInsulineInjected = false;
            LinkedHashMap<PatientType, Integer> map = patientInfoMap;
            map.put(PatientType.HEALTHY, map.get(PatientType.HEALTHY) + map.get(PatientType.FEVER));
            map.put(PatientType.FEVER, 0);
        }

    }

}

The use of map at all places and using static members is looking awkward, so please suggest how I can improve this code as well as object-oriented thinking.

JvR · Accepted Answer · 2017-10-17 16:19:17Z

Reviewing Present Code

This answer is focused on reviewing the code as given, with less focus on the design than the code itself.

PatientType

disease is a code letter, a short-hand; call it code or codeLetter or such.

getValue(PatientType) is convoluted, and it is not clear what 'value' means. Instead, try:

public String getCode() { return code; }

If you expect repeated lookups for codes—and that is probably why you have codes—consider a lookup map:

private static final Map<String, PatientType> codeMap = new HashMap<>();
static {
  for ( PatientType type : values() ) {
    codeMap.put( type.code, type );
  }
}

public static PatientType getByCode(String code) {
  PatientType retval = codeMap.get(code);
  if ( retval == null ) throw new NoSuchElementException(code);
  return retval;
}

ITreatment

Avoid using concrete classes in your method signatures (especially public ones, and doubly for interfaces) when an interface will suffice:

interface ITreatment {
  void treat(Map<PatientType, Integer> patientInfoMap, List<Medicines> appliedTreatmentList);
}

This interface requires some documentation to answer questions implementers will have. What is the expected behavior with respect to the parameters? What do the parameters mean?

At any rate, you will probably want to pass a Quarantine instance. This will help you get rid of the static hacks:

interface ITreatment {
  void treat(Quarantine quarantine);
}

Quarantine

Any time you use a map to link information with an object, consider adding the field directly to the object. If it doesn't belong in the object, or you don't have the ability to make changes, consider a wrapper object, such as a new class Patient.

The statics need to go. Passing the Quarantine object to your Treatments will solve this.

This try-catch seems to be a fail-safe because you're not confident your code is correct:

public String report() {
    try {
        final String[] result = {""};
        patientInfoMap.forEach((k, v) -> result[0] += PatientType.getValue(k) + ":" + v.toString() + " ");
        return result[0].trim();
    } catch (Exception e) {
        e.printStackTrace(); // <--
    }
    return null; // <--
}

... however, if it is incorrect, you will only find out when you try to use the value later and (hopefully) encounter a NullPointerException. Let the exception bubble up.

Try separating the constructor into a method that parses a string and returns a map, and the constructor that creates the object:

protected Quarantine(Map<PatientType, Integer> patientCount);

public Quarantine createFromCodeString(String codeString) {
  Map<PatientType, Integer> patientCount = new LinkedHashMap<>(codeString.length() / 2 + 1);
  for ( String code : codeString.split(",") ) {
    PatientType type = PatientType.getByCode(code);
    patientCount.merge(type, 1, Integer::sum);
  }
  return new Quarantine(patientCount);
}

Now that the logic is more separated out, maybe it would be a better idea to move the parsing itself to PatientType. It's independent from Quarantine:

// PatientType.java
public Map<PatientType, Integer> createFrequencyMap(String codeString) {
  Map<PatientType, Integer> frequency = new HashMap<>(codeString.length() / 2 + 1);
  for ( String code : codeString.split(",") ) {
    PatientType type = getByCode(code);
    frequency.merge(type, 1, Integer::sum);
  }
  return frequency;
}

// Quarantine.java
public Quarantine createFromCodeString(String codeString) {
  return new Quarantine( PatientType.createFrequencyMap(codeString) );
}

TreatmentAspirin

"If the treatments include aspirin and paracetol, then insuline was injected, because they are dead." I feel like I'm missing something crucial to understand this.

@Override
public void treat(LinkedHashMap<PatientType, Integer> patientInfoMap, ArrayList<Medicines> appliedTreatementList) {

    if(appliedTreatementList.contains(Medicines.ASPIRIN)){
        LinkedHashMap<PatientType, Integer> map = patientInfoMap;

        if(appliedTreatementList.contains(Medicines.PARACETOL)){
            Quarantine.isInsulineInjected = true; //Because They are Dead
            map.put(PatientType.DEAD, map.get(PatientType.DEAD) + map.get(PatientType.FEVER) + map.get(PatientType.HEALTHY) + map.get(PatientType.DIABATIC) + map.get(PatientType.TUBERCLOSIS));
            map.put(PatientType.HEALTHY, 0);
            map.put(PatientType.DIABATIC, 0);
            map.put(PatientType.TUBERCLOSIS, 0);
        }else{
            Quarantine.isInsulineInjected = false;
            map.put(PatientType.HEALTHY, map.get(PatientType.HEALTHY) + map.get(PatientType.FEVER));
            map.put(PatientType.FEVER, 0);
        }
    }
}

can you write scalable code for this problem. It would be great help to understand your thought process — Indiver kumar
– Indiver kumar, Commented Oct 26, 2017 at 17:53
@Indiverkumar Do you have questions about the thought process? — JvR
– JvR, Commented Oct 27, 2017 at 15:14
First at all thank you so much for your precious time digging into this. But i just want to see how much scale dynamic(as you mentioned below rule-based-approach) can be applied to this project. The changes you mentioned above i have fixed. But the design you explained below is going out of mind. If would be easier for me to learn by looking into code. If you can give some time that would be much better. For you i have uploaded project on Github github.com/sdindiver/Quaratine — Indiver kumar
– Indiver kumar, Commented Nov 26, 2017 at 20:30

JvR · Accepted Answer · 2017-10-17 16:11:39Z

Reconsidering the Design

The basic principle behind object-oriented design is, put bluntly, to stuff the code where the data is. Store the (source) code with the data it operates on, so that people that reason about the code (that includes the designer!) have to keep fewer things in mind at the same time. Ideally, this leads to robuster and/or simpler code.

If OO is all about putting the code with the data, then how do we decide which code to put with which data?

A good short-hand is to think of objects as having responsibilities. Think of it of asking questions such as: "Who gets to decide what happens?" (who has the code) and "Who knows this?" (who has the data).

Examples:

Who knows what ailments a patient has? (Patient!)
Who knows which patients are in the quarantine? (Quarantine!)
Who decides whether a treatment, when applied to a patient, is effective, ineffective, or lethal? (Hmm...)

Let's define a problem statement, so that we can tell what we are trying to model:

The aim is to model a quarantine. The quarantine has a number of patients. The patients may have ailments / diseases / conditions. The quarantine makes treatments available. Treatments may involve medication.

Note that writing out what we are trying to accomplish already makes some concepts clear. When we model them into classes (object types), we have a few core classes:

Quarantine
  + patients
  ? availableTreatments

Patient
  + ailments

Ailment (Disease, Illness, etc. <- Previously PatientType)

Treatment

Medicine

MedicinalTreatment : Treatment
  + medicine

This is it for data, but what about the code / behavior? We'll go back to the problem statement and refine it:

Quarantine simulates the passage of time. When time passes, (a) treatments applied to patients take effect, and (b) patients are affected by their ailments; some ailments, when untreated when time passes, will kill patients. Treatment is applied to patients. Some treatments include medication. When medication is applied to a patient, it may cure them, have no effect, or kill them, depending on (a) the ailments of the patient, and (b) other medication applied to the patient.

Some behavior looks straightforward. Quarantine controls the passage of time. Time passing is when effects take hold.

The rest is trickier. Medicines may interact with each other and affect the patient. Medicines and ailments may interact and affect the patient. Who gets to decide what?

Quarantine
  + patients
  + tick() (pass time)
    = patients foreach tick()

Patient
  + ailments
  + appliedTreatments
  + tick() (pass time)

Because patients know/own what ailments they have and what treatments were applied to them, they seem ideally placed to decide what happens to them when time passes. Strictly from an OO perspective, this is good design. But the code itself is going to be messy:

public void tick() {
  // am I diabetic? have I been treated with insulin?
  if ( hasAilment(DIABETES) && !isMedicatedWith(INSULIN) ) {
    die(); // x_x
    return;
  }

  // healthy? insulin + antibiotics = fever
  if ( !hasAilments() && isMedicatedWith(INSULIN) && isMedicatedWith(ANTIBIOTICS) ) {
    addAilment(FEVER);
    return;
  }

  // vvv continue ad infinitum
}

This is more of a code maintenance problem: each time we add a medicine, treatment, or ailment, we also need to modify the Patient class. That's because they are tightly coupled in behavior—they depend on each other. For a small enough project, like this one, this is fine. (Larger projects, where there may be many, many more combinations of treatments, medicines, and ailments, will benefit from a rule-based approach, but that feels out of scope here.)

Stack Exchange Network

Object-oriented Quarantine implementation

2 Answers 2

Reviewing Present Code

PatientType

ITreatment

Quarantine

TreatmentAspirin

Reconsidering the Design

You must log in to answer this question.

Hot Network Questions

Object-oriented Quarantine implementation

2 Answers 2

Reviewing Present Code

PatientType

ITreatment

Quarantine

TreatmentAspirin

Reconsidering the Design

You must log in to answer this question.

Related

Hot Network Questions