Fixing the government code

4 min read

OMG WTH is that code?

Recently a tweet about an Argentinian government WebApp code was shared on Twitter and led to a big argument about "code shaming," readability, and seniority. Instead of entering that discussion; let's focus on "fixing" the issues with that code to make it production-ready. First, the problematic code:

if (
	(bodyTemperature >= 38 && difficultyBreathing) ||
	(bodyTemperature >= 38 && difficultyBreathing && diabetes) ||
	(bodyTemperature >= 38 && difficultyBreathing && cancer) ||
	(bodyTemperature >= 38 && difficultyBreathing && isPregnant) ||
	(bodyTemperature >= 38 && difficultyBreathing && isOver60YearsOld) ||
	(bodyTemperature >= 38 && difficultyBreathing && hepatic) ||
	(bodyTemperature >= 38 && difficultyBreathing && kidneyDisease) ||
	(bodyTemperature >= 38 && difficultyBreathing && respiratoryDisease) ||
	(bodyTemperature >= 38 && difficultyBreathing && 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/");
}

Initial optimizations

Initially, we can remove duplicated lines and duplicated logic:

if (
	(bodyTemperature >= 38 && difficultyBreathing) ||
	(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, it is a little bit easier to read. Now, let's focus on turning that multi-line AND/OR into a nested if structure:

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

That's an improvement, but we can do even better. Let's move some of the logic into constants with names that allow the developer to understand what's happening:

const hasFever = bodyTemperature >= 38;
const hasOtherSymptoms =
	difficultyBreathing ||
	diabetes ||
	cancer ||
	isPregnant ||
	isOver60YearsOld ||
	hepatic ||
	kidneyDisease ||
	respiratoryDisease;

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

Taking it one step further

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

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

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

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

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

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

export const needsAttention = patient =>
	hasFever(patient) && hasOtherSymptoms(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 places where we need this logic, we can call redirectPatient(patient, history); and done!

The things we should be discussing

We could come up with many different solutions, but I believe the main thing we should be discussing is:

Public government apps should be open source and accept "pull requests" from developers to be improved. Especially if it implies public health.

And before discarding this as a "political" post, I'm just pointing out that government apps, as anything government-related thing, should be "democratic" and "auditable," and there isn't anything more democratic than open source.

Share this article