3
\$\begingroup\$

I just started an internship and we have a little task to do. It should follow the SOLID principles and I have a slight idea, but I'm not exactly sure what I'm looking for and if you can tell me how to improve my code you will help me a lot. Also, why is the forEach not working and I have to make a For loop?

This is the task: Imagine a medical app for registering patients. As medical worker I want to input 3 patients in the system(first name, last name, date of birth) in random order. After the 3rd patient when I press Enter a list of the patients is returned in sorted, alphabetical order. You have the lecture, keep in mind the S.O.L.I.D principles, code reusability etc.

P.S. There are always 3 patients.

// let list = prompt('Enter list of patients.');
let list = 'John, Doe, 18/05/1995;Jane, Doe, 13/12/1988;Brett, Cooper, 05/11/1991;';
list = list.split(';');

class Patient {

    constructor(fname, lname, date) {
        this.fname = fname;
        this.lname = lname;
        this.date = date;
    }

    loader(patients) {
        for(let i = 0; i < 3; i++) {
            const tempArr = list[i].split(',');

            // tempArr.forEach(e => e = e.trim());
            for(let i = 0; i < tempArr.length; i++) {
                tempArr[i] = tempArr[i].trim();
            }

            patients.push(new Patient(tempArr[0], tempArr[1], tempArr[2]));
        }
    };
}

function objSort(arr) {
    arr.sort((a,b) => (a.fname > b.fname) ? 1 : ((b.fname > a.fname) ? -1 : 0));
}

let patients = [];
let patient = new Patient();

patient.loader(patients);
objSort(patients);

let listForPrint = '';
patients.forEach(patient => {
    listForPrint += `${patient.fname} ${patient.lname} ${patient.date}\n`;
});

console.log(listForPrint);
\$\endgroup\$
3
  • \$\begingroup\$ Welcome to Code Review. The title in your original question raises a very common concern and is therefore inappropriate for this site. Our convention is to state the task accomplished by the code — see How to Ask. \$\endgroup\$ Aug 3 at 22:08
  • \$\begingroup\$ SOLID principles are guidelines for object-oriented programming. Your code is only mildly object-oriented, so it's not SOLID by definition. \$\endgroup\$ Aug 3 at 22:11
  • \$\begingroup\$ why is the forEach not working Only working code is appropriate for Code Review. Fixing broken code changes the code. Changing code necessarily changes its evaluation in the context of SOLID principles. \$\endgroup\$
    – radarbob
    Aug 4 at 3:23

2 Answers 2

3
\$\begingroup\$
  1. You need a class that will responsible for functionality related to the system/medical app.

  2. You need a class that will responsible for loading patients from data.

The class to represent the system/medical app should have a property to keep the list of patients, a method to add a patient, and a method to get all the patients, and perhaps, a method to print all the patients.

After the method addPatient is called, you can sort the list of patients.

Example:

class MedicalSystem {
    #patients = []; // this will hold instances of Patient

    addPatient(firstName, lastName, ...) {
        // Add the patient to the patients list
        // sort the list
    }

    getPatients() {
        // return all the patients, which are all sorted
    }

    printPatients() {
        // print the patients
    }
}

All the fields in the Patient class should be marked private. It should have getters and setters to expose/update the private fields.

Also move the loader method out of the Patient class. Create a separate class to load patients from data.

Example:

class PatientDataLoader {
   // add a variable here to keep the list of loaded patients.

   constructor(data) {
      // load/parse patient data
   }
   
   getPatients() {
     // returns the loaded patients
   }
}
\$\endgroup\$
1
  • \$\begingroup\$ printPatients() locks the output to the particular IO coded - terminal, web console, web UI/Page... A single responsibility violation I'd say. Perhaps Open/Closed as well. Let the object that wants to output the list do it. Give that object access to the patient collection withgetPatients(), that's all. \$\endgroup\$
    – radarbob
    Aug 4 at 19:01
2
\$\begingroup\$

It should follow the SOLID principles and I have a slight idea, but I'm not exactly sure what I'm looking for

SOLID does not tell us how to design an object oriented system. SOLID helps us evaluate the design so it has the desirable traits of maintainable code.

You must see that a Patient and the Patients are different things, have different functionality (responsibilities) and should be different classes. Otherwise you will not see this as a violation of Single Responsibility Principle.


Real world common sense can help.

If you are a hospital patient, do you personally have a list of all hospital patients? Of course not, It's not your responsibility.

If the hospital wants to print your name in a certain format for a name card, are you, the patient, expected to provide your name in that format? Heck no! Why is an admin clerk asking me to do his job? I'm the one who's sick - GET OUT OF MY ROOM (class)! Here is my name, now leave me alone.

If the hospital wants a sorted patient list, the Patients class is the reasonable place to put that responsibility. As an implementation issue the Patients collection class knows its own internal data structures and thus can easily sort itself. Other classes should not have to know such details; indeed they should not be allowed to. Only "Hey, you there! mentalPatients.sort()".


All the above is all about the S in SOLID. That is by far most important of the SOLIDs. Unfortunately the "Single Responsibility" technical definition alone does not give one any practical insight for the specific design of a specific design problem.

\$\endgroup\$

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.