5
\$\begingroup\$

I have an assignment to implemented a Quarantine project where I was left with the some unit tests and a skeleton of the implementation. I provided the solution below and get rejected from the interview. The feedbacks I get (I feel lucky that they provided, usually, they have legal issues) are,

  1. NO indicator of design (DDD, or design patterns)
  2. The code is not scalable (extendable)

Overall, this is C code written in Java (bit harsh and made me sad). Is there any better way to do that? I mainly looking suggestions from the very experienced engineers.

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());
    }

}


import java.util.LinkedHashMap;
import java.util.Map;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

public class Quarantine {

    private Map<Character, Integer> map;

    boolean insuline;
    boolean wait40Days;
    boolean antibiotic;
    boolean aspirin;
    boolean paracetamol;
    public Quarantine(String subjects) {
        try {
            map = Pattern.compile(",")
                    .splitAsStream(subjects)
                    .collect(Collectors.groupingBy(
                            s -> s.charAt(0),
                            LinkedHashMap::new,
                            Collectors.collectingAndThen(Collectors.counting(), Long::intValue)
                    ));
            map.put('X', 0);
        } catch (Exception e) {
            e.printStackTrace();
        }
    }

    // aspirin cures fever
    public void aspirin() {
        aspirin = true;
    }

    public void antibiotic() {
        antibiotic = true;
    }

    public void insulin() {
        insuline = true;
    }

    public void paracetamol() {
        paracetamol = true;
    }

    public void wait40Days() {
        if (antibiotic) {
            // antobiotic is mixed with the insuline
            if (insuline) {
                map.put('F', map.get('F') + map.get('H'));
                map.put('H', map.get('T'));
                map.put('T', 0);
                return;
            }
            // only the antibiotic
            else {
                map.put('H', map.get('H') + map.get('T'));
                map.put('T', 0);
                wait40Days = true;
            }
        } else if (paracetamol) {
            // paracetamol mixed with the aspirin kills everyone
            if (aspirin) {
                map.put('X', map.get('X') + map.get('F') + map.get('H') + map.get('D') + map.get('T'));
                map.put('F', 0);
                map.put('H', 0);
                map.put('D', 0);
                map.put('T', 0);
                return;
            } else { // only provides the paracetamol as medication
                map.put('H', map.get('H') + map.get('F'));
                map.put('F', 0);
                wait40Days = true;
            }
        } else if(aspirin) { // only provides aspirin as medication
            map.put('H', map.get('H') + map.get('F'));
            map.put('F', 0);
            wait40Days = true;
        } else if (insuline) {
            // only provision of insuline prevents death from the diabetes
            return;
        } else {         // no medicine was provided, just waited for the 40 days
            wait40Days = true;
        }

        /*
        check if we will needs to wait for 40 days
        after the medication to see the affect
        * */
        if (wait40Days) {
            map.put('X', map.get('D'));
            map.put('D', 0);
            wait40Days = false;
        }
    }

    // get the Quarantine report
    public String report() {

        try {
            final String[] result = {""};
            map.forEach((k, v) -> result[0] += k.toString() + ":" + v.toString() + " ");
            return result[0].trim();
        } catch (Exception e) {
            e.printStackTrace();
        }
        return null;
    }
}
\$\endgroup\$
  • \$\begingroup\$ I think you can split wait40Days in more small methods. \$\endgroup\$ – alayor Feb 9 '17 at 5:30
  • \$\begingroup\$ Please, read the question again. I need better design patterns for this implementation. \$\endgroup\$ – Arefe Feb 9 '17 at 7:53
  • 1
    \$\begingroup\$ This is missing anything resembling OO. Immediately, I'd create a Patient class to monitor a persons health, and a class hierarchy of Treatments with sub-classes for aspirin, paracetamol, etc. Somehow model the interaction between patient and treatment, have a quarantine class, which acts as a factory for treatments and as control object, etc. Think about additional treatment being added: if a new treatment "voodomagic" gets added, how can you incorporate this without changing existing code? \$\endgroup\$ – mtj Feb 9 '17 at 11:40
  • \$\begingroup\$ @mtj could be please provide some code for me ? I started learning design patterns properly now and it will be good motivation for me. Thanks. \$\endgroup\$ – Arefe Feb 9 '17 at 13:18
  • \$\begingroup\$ @mtj Ihave a question: you said have a quarantine class, which acts as a factory for treatments and as control object, etc, do you mean I need to use Factory design patterns for the Quarantine class ? I will see the factory in the night and will try to implement the code. \$\endgroup\$ – Arefe Feb 9 '17 at 13:25
1
\$\begingroup\$

So yes, your code lack of Object Oriented design and patterns. A quick look give me the idea that the Quarantine can be a factory for Medicine, but that is a little improvement.

The first thing to do is to create your models. You have the Quarantine class but also a group of Patients, some Medecine and a set of Disease.

The Disease are a facts, something that you will just accept, so it can be an enumeration. [pattern enumeration]

The Patients maintains a counter, that can move from one group to the other. It just maintains an integer an can be seen as a decorator. [pattern decorator]

class Patients {
   int size;
   void becomes(Patients other) {
     other.size += this.size;
     this.size = 0;
   }
}

The Quarantine distribute one or more Medicine to all the Patients, I have created a Treatment class to maintains all the Medicine. This class is immutable and allows me to query it and add or remove Medicine. The quarantine can be a factory and a builder (because it compose the treatment).

The new Treatment class can be seen as many patterns, helper or state]. But also a composite because it implements the Medicine but is also a delegate.

Finally, the biggest part is from the Medicine where all the logic reside. When add to the Treatment a Medicine will change the results of it. So a Medicine must be combined with an existing Treatment and given to a group of Patients to produce his effects. The strategy pattern apply to this class.

interface Medicine {
  void on(Quarantine quarantine);
  Treatment combine(Treatment treatment);
}
\$\endgroup\$
  • \$\begingroup\$ Would you mind to provide little more complete code if you have time? I really appreciate the answer anyway. \$\endgroup\$ – Arefe Feb 15 '17 at 7:42
  • 1
    \$\begingroup\$ Here you are : github.com/gervaisb/stackexchange-codereview/tree/master/src/… (There is also some tests into `src/test/java/q154869). \$\endgroup\$ – gervais.b Feb 15 '17 at 9:04
  • \$\begingroup\$ Hey, I like the code you wrote. Great solution. \$\endgroup\$ – Arefe Feb 17 '17 at 2:29
  • \$\begingroup\$ One of the test cases is failing and I out a question in the Stackoverflow, stackoverflow.com/questions/42291738/… \$\endgroup\$ – Arefe Feb 17 '17 at 7:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.