3
\$\begingroup\$

For my students (I am an Austrian math teacher) I would like to provide some simple online calculators on my blog.

Let's say there are two pages on my website("Circle" and "Cube") and on every page I want to have about 10 different calculators.

Here is an example of one page with two calculators:

enter image description here

My code

JSFiddle

HTML

<h1>Circle</h1>

<h2>Calculate circumference of circle</h2>
<p>
 Radius of circle: <input type="text" id="calc1-input"> <button onclick="calc1()">Calculate</button>
</p>
<p>
Circumference of circle: <span id="calc1-output"></span>
</p>

<h2>Calculate area of circle</h2>
<p>
 Radius of circle: <input type="text" id="calc2-input"> <button onclick="calc2()">Calculate</button>
</p>
<p>
Area of circle: <span id="calc2-output"></span>
</p>

Javascript

function commaToDot(number) {
    return parseFloat(number.replace(',','.').replace(' ',''));
}

function dotToComma(number){
    return number.toString().replace('.',',');
}

function calc1() {
    var input = document.getElementById("calc1-input").value;
    input = commaToDot(input);
    if (isNaN(input)) {
        document.getElementById('calc1-output').innerHTML = "<b>Please enter a number!</b>";
    } else {
      var output = 2 * Math.PI * input;
        document.getElementById('calc1-output').innerHTML = dotToComma(output);
    }
}

function calc2() {
    var input = document.getElementById("calc2-input").value;
    input = commaToDot(input);
    if (isNaN(input)) {
        document.getElementById('calc2-output').innerHTML = "<b>Please enter a number!</b>";
    } else {
      var output = Math.PI * Math.pow(input,2);
        document.getElementById('calc2-output').innerHTML = dotToComma(output);
    }
}

Explanation

  • Input: In my country we use commas as decimal separators.
    So the students write 55,5 instead of 55.5 (see commaToDot()).
  • Output: The students expect the output to be with comma decimal separator, too (see dotToComma()).
  • Error: If the input is not a number (if (isNaN(input))) an error message will be displayed instead of the calculation result.

Questions

  • I see a lot of redundance in my code but I don't know how to improve it.
    The only difference between calc1() and calc2() is the formula.
  • Maybe it is possible to improve the naming of the variables/functions?
\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

calc1 and calc2 are really bad names. Names should always clearly describe what they are for or do. calculateCircleCircumference and calculateCircleArea would be better choices.

I'm not a big fan of the names commaToDot and commaToDot either. I'd prefer more conceptional names such as parseNumber and formatNumber, maybe even parseGermanNumber and formatGermanNumber.

A slightly better HTML structure would be in order: Each calculator could be surrounded by a <form> element and there is an <output> element specifically for display calculation results.

In current JavaScript one should prefer const and let over var.

The following suggestions may introduce concepts, that the students haven't learnt yet, but they are considered good conventions.

For a better separation of layout and logic it is usually suggested not to use on... event handler attributes, but instead assign them using addEventListener.

Also one should move all element look ups to the initialization. This could look like this (notice the changed IDs):

const circleCircumferenceRadiusInput = document.getElementById("circle-circumference-radius-input");
const circleCircumferenceOutput = document.getElementById('circle-circumference-output');

function calculateCircleCircumference() {
    // Using separate variables so "const" can be used.
    const inputString = circleCircumferenceRadiusInput.value;
    const input = commaToDot(inputString);
    if (isNaN(input)) {
      circleCircumferenceOutput.innerHTML = "<b>Please enter a number!</b>";
    } else {
      const output = 2 * Math.PI * input;
      circleCircumferenceOutput.innerHTML = dotToComma(output);
    }
}

document.getElementById('circle-circumference-button').addEventHandler("click", calculateCircleCircumference);

This can be simplified a bit by wrapping HTML of the calculator in an <form> element and referring to the needed elements by name. Also I'm wrapping the code in an initialization function to avoid lots of global variables. Both of these prepare for generalizing and reusing the code.

<form id="circle-circumference">
  <h2>Calculate circumference of circle</h2>
  <p>
    Radius of circle: <input type="text" name="radius"> <button name="execute">Calculate</button>
  </p>
  <p>
    Circumference of circle: <output name="output"></output>
  </p>
</form>
function initCircleCircumferenceCalculator(form) {
  const radiusInput = form.elements["radius"];
  const outputElement = form.elements["output"];

  function calculate() {
    const inputString = radiusInput.value;
    const input = commaToDot(inputString);
    if (isNaN(input)) {
      outputElement.innerHTML = "<b>Please enter a number!</b>";
    } else {
      const output = 2 * Math.PI * input;
      outputElement.innerHTML = dotToComma(output);
    }
  }

  form.elements["execute"].addEventHandler("click", calculate);
} 

initCircleCircumferenceCalculator(document.getElementById("circle-circumference"));

Now that we have the initialization function it's easier to generalize this so that it can be used for multiple calculators:

function initCalculator(form, inputName, calculationFunction) {
  const inputElement = form.elements[inputName];
  const outputElement = form.elements["output"];

  function calculate() {
    const inputString = inputElement.value;
    const input = commaToDot(inputString);
    if (isNaN(input)) {
      outputElement.innerHTML = "<b>Please enter a number!</b>";
    } else {
      const output = calculationFunction(input);
      outputElement.innerHTML = dotToComma(output);
    }
  }

  form.elements["execute"].addEventHandler("click", calculate);
} 

// Circle circumference
initCalculator(document.getElementById("circle-circumference"), "radius", r => 2 * Math.PI * r);

// Circle area 
initCalculator(document.getElementById("circle-area"), "radius", r => Math.PI * Math.pow(r, 2));

The next step could be modify initCalculator so that it supports multiple input fields.

\$\endgroup\$
1
  • \$\begingroup\$ That's really impressive. Thanks for sharing your skill with me. Maybe you can add the following feature: There are calculators with two or more inputs. I think an array will do the job? \$\endgroup\$ Commented Jun 29, 2020 at 12:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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