Small Bayes Calculator to learn Javascript

Question

I am trying to learn UI design so I threw together a Bayes Calculator (i.e. calculate the posterior probability of being sick given a positive/negative test result using the specificity, sensitivity of the test and current incidence rate). Since I already wrote markdown for a static site generator (hugo) I similarly created a html content page which is wrapped by the hugo theme fuji. This theme did not include styling for input elements (likely because it is intended for blogpost like markdown and not html). So I had to add some custom SCSS styling (but using existing color variables). In particular I added some logic to only activate form validation styling once an input element was modified once (add the class "wasModified").

The end result looks like this:

enter image description here

I am unsure how to align the input elements. A table would work, but then I lose the wrap around on mobile putting the labels above the input fields. Are there any best practices?

HTML

<!--content/blog/tools/bayes-helper.html -->
---
title: Bayes Helper
date: "2022年01月22日"
tags: [maths, tools, health]
draft: true
---
<form id="bayes-form">
 <label for="incidence">Incidence Rate</label>
 <input type="number", id="incidence", name="incidence", min="0", max="100000", step="1", placeholder="per 100 000", required> 
 <br>
 <label for="sensitivity">Sensitivity</label>
 <input type="number", id="sensitivity", name="sensitivity", min="0", max="100", step=".01", placeholder="in percent", required> 
 <br>
 <label for="specificity">Specificity</label>
 <input type="number", id="specificity", name="specificity", min="0", max="100", step=".01", placeholder="in percent", required> 
 <br>
 <label for="test_positive">Test positive?</label>
 <input type="checkbox", id="test_positive", name="test_positive"> 
 <br>
 <label for="posterior" >Likelihood to be infected:</label>
 <output id="posterior" name="posterior">Input not Valid</output>
</form>
<script>
 function _calculatePosterior(prior, sensitivity, specificity, test_positive) {
 const p_test_given_pos = test_positive ? sensitivity : 1-sensitivity
 if (prior==0 || p_test_given_pos == 0){
 return 0
 }
 const p_test_given_neg = test_positive ? 1-specificity : specificity
 return 1/(1+ p_test_given_neg*(1-prior)/(p_test_given_pos*prior))
 }
 function calculatePosterior() {
 this.posterior.value = _calculatePosterior(
 this.incidence.value/100000.0,
 this.sensitivity.value/100.0,
 this.specificity.value/100.0,
 this.test_positive.checked
 ) *100 + "%";
 }
 document.getElementById("bayes-form").oninput = calculatePosterior
 document.getElementById("bayes-form").onreset = calculatePosterior
 function add_wasModified() {
 this.classList.add("wasModified");
 }
 for (const input of document.getElementsByTagName("input")) {
 input.oninput = add_wasModified;
 }
</script>

SCSS

// assets/scss/_custom_rules.scss
// Override CSS rules with that file
input {
 color: var(--color-font);
 background-color: var(--color-bg);
 border: 2px solid var(--color-divider);
 border-radius: 2px;
 padding: 5px;
 margin: 10px;
 &:hover {
 border: 2px solid var(--color-primary);
 };
 accent-color: var(--color-primary);
}
input.wasModified:invalid {
 border: 2px solid $border-red-light;
}
button {
 color: var(--color-font);
 background-color: var(--color-codebg);
 border-color: var(--color-primary);
 border-radius: 2px;
}

Appendix: Math

For those interested (on request). So "sensitivity" is the likelihood of a test being positive conditional on truly having the condition, let us denote this by $$P(\text{test_p}|\text{pos})$$ "Specificity" is the likelihood of a test being negative conditional on not having the condition (being negative), denoted by $$P(\text{test_n} | \text{neg})$$

Now we want to know the probability of having the condition conditional on the test result, i.e.

$$\begin{align} P(\text{pos}|\text{test}) &= \frac{P(\text{pos}, \text{test})}{P(\text{test})} = \frac{P(\text{test}| \text{pos}) P(\text{pos})}{P(\text{test}, \text{pos}) +P(\text{test},\text{neg})}\\ &=\frac{ P(\text{test}| \text{pos})P(\text{pos}) }{ P(\text{test}| \text{pos})P(\text{pos}) +P(\text{test}|\text{neg})(1-P(\text{pos})) }\\ &=\frac{1}{1+\frac{ P(\text{test}|\text{neg})(1-P(\text{pos})) }{ P(\text{test}| \text{pos})P(\text{pos})} } \end{align}$$

Okay so what do we need to calculate this. First of all we need the prior probability to have the condition (before doing the test), P(pos), this is the incidence rate. For the two conditional probabilities we need we just need to look at specificity and sensitivity. For a positive test result we have

$$\begin{align} P(\text{test} | \text{pos}) &= P(\text{test_p} | \text{pos}) =\text{specificity}\\ P(\text{test} | \text{neg}) &= P(\text{test_p} | \text{neg}) = P(\text{test_n} | \text{neg}) = 1-\text{sensitivity} \end{align}$$ for a negative test result we have $$\begin{align} P(\text{test} | \text{pos}) &= P(\text{test_n} | \text{pos}) = 1- \text{specificity}\\ P(\text{test} | \text{neg}) &= P(\text{test_n} | \text{neg}) = \text{sensitivity} \end{align}$$

This explains the ternary operator. Then you just need to do some considerations for the probability zero cases and you just need to type up the algorithm.

• coderodde: Could you consider posting the actual math? I believe many users might be interested in it.
• Felix Benning: @coderodde I added an appendix (as a picture because stackexchange detects latex as code and complains that I do not use code fences)
• Felix Benning: wrt to LaTeX see: codereview.meta.stackexchange.com/questions/10757/…

Answer 1

UX

Overall, fine. Good use of labels.

Getting an invalid input error off the bat is a bit jarring.

Consider formatting the result to a readable number of decimal places (say, 2) unless you have good reason to be more precise.

Code style

In general, try to stick to conventions and be consistent.

JS

• JS uses camelCase rather than snake_case (p_test_given_pos). add_wasModified uses a mix and isn't a style I see much of in JS.
• Add space between operators: if (prior==0 || p_test_given_pos == 0){ should have prior == 0. return 1/(1+ p_test_given_neg*(1-prior)/(p_test_given_pos*prior)) is clearer with intermediate variables and spaces around operators. These are good variable names, though.
• Use === always. == does confusing type coercions so it can easily lead to bugs. Always using === eliminates all of these bugs and improves readability by making intent explicit (explicitly coerce when necessary).
• Stick to using semicolons after every statement or skip them entirely except when necessary. It's more common to use them. Once again, it eliminates a whole class of bugs, quickly becomes second nature, and isn't really less "clean" looking in my opinion. I doubt it impacts development speed meaningfully. See Do you recommend using semicolons after every statement in JavaScript?
• I'm not sure why _calculatePosterior has a leading underscore. I guess because it's a "private" helper function for calculatePosterior, which does DOM interaction and formatting. It's good to separate the two, but maybe choose clearer variable names. For the handler, you can use handlePosteriorRecalculation (since it's an event handler) and calculatePosterior for the non-I/O math. I typically see leading underscores for private class methods and variables, neither of which applies here.
• JS usually uses 2-space indentation. I see 4 from time to time, so it's a minor point.

CSS

• CSS names usually use kebab-case, so wasModified would be was-modified (I'd just call it modified though). But you got it right with bayes-form (although "form" might be redundant -- maybe form.bayes-calculator is clearer? I don't know the best practice here).
• Your SCSS structure looks fine but perhaps move accent-color: var(--color-primary); up above the &hover block.
• In a larger app, scope your CSS selectors specifically: .bayes-calculator input instead of input.

HTML

• In the HTML, type="number", id="incidence", ... shouldn't have commas.

• I don't like long HTML lines for the same reason as long code lines. I suggest:

  <input
   type="number"
   id="incidence"
   name="incidence"
   min="0"
   max="100000"
   step="1"
   placeholder="per 100 000"
   required
  > 
  

• Don't use <br> for styling; it's inflexible and semantically conflates structure (HTML) with style (CSS). Prefer CSS's margin-bottom and display: block or similar. If these are block lines, wrap them in <div>s or wrap the <label> around the <input> instead of using for=. <br> is only for breaks in <p> elements, as in an address. See When to use <p> vs. <br>.

Code design

There's not much of an app to speak of design, so this is premature, but...

Usually, functions with a flag that switches between multiple behaviors is poor design. This has been given comprehensive treatment elsewhere, so I'll just point the way: Is it wrong to use a boolean parameter to determine behavior?. TL;DR quote from the top answer in the link:

Yes, this is likely a code smell, which would lead to unmaintainable code that is difficult to understand and that has a lower chance of being easily re-used.

The solution is to break positive and negative into two separate functions, then call a third function for the shared math logic to keep that DRY.

Your use of oninput and onreset has a thread on Stack Overflow as well: addEventListener vs onclick. I'd prefer addEventListener, but oninput seems acceptable here. I also prefer not to use this to refer to a form.

---

Possible rewrite

const calculatePosterior = (prior, pTestGivenPos, pTestGivenNeg) =>
 prior === 0 || pTestGivenPos === 0 ? 0
 : 1 / (1 + pTestGivenNeg * (1 - prior) / (pTestGivenPos * prior))
;
const calculatePositivePosterior = (prior, sensitivity, specificity) =>
 calculatePosterior(prior, sensitivity, 1 - specificity)
;
const calculateNegativePosterior = (prior, sensitivity, specificity) =>
 calculatePosterior(prior, 1 - sensitivity, specificity)
;
const handlePosteriorRecalculation = ({target: {form}}) => {
 const posteriorFn = form["test-positive"].checked
 ? calculatePositivePosterior
 : calculateNegativePosterior
 ;
 form.posterior.value = posteriorFn(
 +form.incidence.value / 100000,
 +form.sensitivity.value / 100,
 +form.specificity.value / 100,
 ) * 100 + "%";
};
const bayesForm = document.getElementById("bayes-form");
bayesForm.addEventListener("input", handlePosteriorRecalculation);
bayesForm.addEventListener("reset", handlePosteriorRecalculation);
for (const input of document.getElementsByTagName("input")) {
 input.addEventListener("input", () => input.classList.add("modified"));
}

<form id="bayes-form">
 <div>
 <label for="incidence">Incidence Rate</label>
 <input
 type="number"
 name="incidence"
 id="incidence"
 min="0"
 max="100000"
 step="1"
 placeholder="per 100 000"
 required
 >
 </div>
 <div>
 <label>Sensitivity
 <input
 type="number"
 id="sensitivity"
 name="sensitivity"
 min="0"
 max="100"
 step=".01"
 placeholder="in percent"
 required
 >
 </label>
 </div>
 <div>
 <label for="specificity">Specificity</label>
 <input
 type="number"
 name="specificity"
 id="specificity"
 min="0"
 max="100"
 step=".01"
 placeholder="in percent"
 required
 >
 </div>
 <div>
 <label for="test-positive">Test positive?</label>
 <input
 type="checkbox"
 name="test-positive"
 id="test-positive"
 >
 </div>
 <div>
 <label for="posterior">Likelihood to be infected:</label>
 <output name="posterior" id="posterior">
 Input not Valid
 </output>
 </div>
</form>

• Felix Benning: I do not quite understand the this.posterior.value comment, as far as I understood quirksmode.org/js/this.html this refers to its owner (in this case the form and not the entire window). I thought that this was a better idea because it uses in some sense local variables of the form instead of doing a global lookup with getElementById which seems like a weird practice coming from other programming languages.
• Felix Benning: thank you for the link to the addEventListener vs onclick comparison, I did not think about the fact that onclick does not allow multiple listeners and is therefore not a good idea
• ggorlen: Yep, you're right! My mistake. Removed.
• Felix Benning: I am not sure if I would call the test_positive variable behaviour determining variable. In both cases I am calculating the posterior probability of being infected. So I am not changing the semantic control flow. Sure I need to handle those cases differently, but that is due to a mathematical technicality which I wanted to abstract away with _calculatePosterior. The fact that I only need to swap out 1-specificity for specificity is an extremely technical detail which should not concern people using this function.
• ggorlen: @IsmaelMiguel I appreciate the feedback! My style is more "modern" ES6 than OP's, which doesn't appeal to everyone immediately. The semicolons terminate the function statements. It'd be weird for me to recommend semicolons after every statement, then not use them. You can move them up a line if you want. The + is for type conversion, so it's a unary operator--the spacing only applies to binary ops. Regarding number conversion, I didn't want to change the functionality for my rewrite, so it's left as a suggestion, and there are plenty of resources on how to do that, if desired.