loading...

Fixing the gov code...

lukeshiru profile image ▲ LUKE知る ・2 min read

Inspiration:

Recently a tweet about some argentinian gov code was shared, and led to a big discussion about "code shaming", readability and seniority. I'll not enter the discussion, but rather focus on "fixing" the issues with that code to make it production ready. So the initial code is this one:

if (
    (bodyTemperature >= 38 && diffultyBreathing) ||
    (bodyTemperature >= 38 && diffultyBreathing && diabetes) ||
    (bodyTemperature >= 38 && diffultyBreathing && cancer) ||
    (bodyTemperature >= 38 && diffultyBreathing && isPregnant) ||
    (bodyTemperature >= 38 && diffultyBreathing && isOver60YearsOld) ||
    (bodyTemperature >= 38 && diffultyBreathing && hepatic) ||
    (bodyTemperature >= 38 && diffultyBreathing && kidneyDisease) ||
    (bodyTemperature >= 38 && diffultyBreathing && respiratoryDisease) ||
    (bodyTemperature >= 38 && diffultyBreathing && respiratoryDisease) ||
    (bodyTemperature >= 38 && diabetes) ||
    (bodyTemperature >= 38 && cancer) ||
    (bodyTemperature >= 38 && isPregnant) ||
    (bodyTemperature >= 38 && isOver60YearsOld) ||
    (bodyTemperature >= 38 && hepatic) ||
    (bodyTemperature >= 38 && kidneyDisease) ||
    (bodyTemperature >= 38 && respiratoryDisease) ||
    (bodyTemperature >= 38 && respiratoryDisease)
) {
    history.replace(`/diagnostico/${provincia}`);
} else if (bodyTemperature >= 38) {
    history.replace('/cuarentena/');
} else if (bodyTemperature < 38) {
    history.push('/diagnostico_bueno/');
} else {
    history.push('/diagnostico_bueno/');
}

First, lets just remove duplicated lines and duplicated logic:

if (
    (bodyTemperature >= 38 && diffultyBreathing) ||
    (bodyTemperature >= 38 && diabetes) ||
    (bodyTemperature >= 38 && cancer) ||
    (bodyTemperature >= 38 && isPregnant) ||
    (bodyTemperature >= 38 && isOver60YearsOld) ||
    (bodyTemperature >= 38 && hepatic) ||
    (bodyTemperature >= 38 && kidneyDisease) ||
    (bodyTemperature >= 38 && respiratoryDisease)
) {
    history.replace(`/diagnostico/${provincia}`);
} else if (bodyTemperature >= 38) {
    history.replace('/cuarentena/');
} else {
    history.push('/diagnostico_bueno/');
}

Without the bloat, is a little bit easier to read. Now, let's focus in turning that multiple line AND+OR, into a nested if logic for now:

if (bodyTemperature >= 38) {
    if (
        diffultyBreathing ||
        diabetes ||
        cancer ||
        isPregnant ||
        isOver60YearsOld ||
        hepatic ||
        kidneyDisease ||
        respiratoryDisease
    ) {
        history.replace(`/diagnostico/${provincia}`);
    } else {
        history.replace('/cuarentena/');
    }
} else {
    history.push('/diagnostico_bueno/');
}

That is still kinda hard to read, right? Let's keep optimizing, now we can move some of the logic into constants with names that allow the developer to understand what's happening:

const hasFever = bodyTemperature >= 38
const hasExtraSymptoms = 
    diffultyBreathing ||
    diabetes ||
    cancer ||
    isPregnant ||
    isOver60YearsOld ||
    hepatic ||
    kidneyDisease ||
    respiratoryDisease;

if (hasFever) {
    if (hasExtraSymptoms) {
        history.replace(`/diagnostico/${provincia}`);
    } else {
        history.replace('/cuarentena/');
    }
} else {
    history.push('/diagnostico_bueno/');
}

That's super readable and far better than the initial implementation. We could go one step further, turning this into a fully functional approach, but that might not be super easy to read for some folk, so this last step is "optional". Still, if you separate that logic into several files, it becomes far easier to maintain and test:

// hasFever.js
export const hasFever = ({ bodyTemperature }) => bodyTemperature >= 38

// hasExtraSymptoms.js
export const hasExtraSymptoms = patient =>
    patient.diffultyBreathing ||
    patient.diabetes ||
    patient.cancer ||
    patient.isPregnant ||
    patient.isOver60YearsOld ||
    patient.hepatic ||
    patient.kidneyDisease ||
    patient.respiratoryDisease;

// needsQuarentine.js
import { hasFever } from "./hasFever.js";
import { hasExtraSymptoms } from "./hasExtraSymptoms.js";

export const needsQuarentine = patient => hasFever(patient) && !hasExtraSymptoms(patient);

// needsAttention.js
import { hasFever } from "./hasFever.js";
import { hasExtraSymptoms } from "./hasExtraSymptoms.js";

export const needsAttention = patient => hasFever(patient) && hasExtraSymptoms(patient);

// redirectPatient.js
import { needsAttention } from "./needsAttention";
import { needsQuarentine } from "./needsQuarentine";

export const redirectPatient = (patient, history) => history.push(
    needsAttention(patient)
        ? `/diagnostico/${provincia}`
        : needsQuarentine(patient)
            ? '/cuarentena/'
            : '/diagnostico_bueno/'
);

So in the actual place where you need this, you just call redirectPatient(patient, history); and you're done.

That's it from me, you obviously can come up with even better solutions, but I think the discussion now should be: Public government apps should be open source and should accept pull requests from developers to improve constantly.

Thanks for taking the time to read this!

Posted on by:

Discussion

markdown guide
 

I have published an article with this same topic, dev.to/damxipo/refactoring-the-dia... hahahah you are from Argentina?

 

I'm living in the US now, but I'm from Argentina, yup 🤣