HTML/JavaScript temperature converter with rounding

Question

I am looking to improve the performance of my code through best practices and cleaner code.

My goal here is just to perform a conversion whenever there is a keyup.

But I noticed that my code looks very repetitive. In your view, what should I do to simplify it?

window.oninput = function(event) {
 var campo = event.target.id;
 // DECIMALS
 var i = document.getElementById("decimalTemp").value;
 oC.value = (oC.value * 1).toFixed(i);
 oK.value = (oK.value * 1).toFixed(i);
 oF.value = (oF.value * 1).toFixed(i);
 oRa.value = (oRa.value * 1).toFixed(i);
 // TEMPERATURE
 if (campo == "oC") {
 oK.value = (oC.value * 1 + 273.15).toFixed(i);
 oF.value = (oC.value * 1.8 + 32).toFixed(i);
 oRa.value = ((oC.value * 1 + 273.15) * 1.8).toFixed(i);
 } else if (campo == "oK") {
 oC.value = (oK.value * 1 - 273.15).toFixed(i);
 oF.value = (oK.value * 1.8 - 459.889).toFixed(i);
 oRa.value = (oK.value * 1.8).toFixed(i);
 } else if (campo == "oF") {
 oC.value = ((oF.value * 1 - 32) / 1.8).toFixed(i);
 oK.value = ((oF.value * 1 + 459.67) / 1.8).toFixed(i);
 oRa.value = (oF.value * 1 + 459.67).toFixed(i);
 } else if (campo == "oRa") {
 oC.value = (oRa.value / 1.8 - 273.15).toFixed(i);
 oK.value = (oRa.value / 1.8).toFixed(i);
 oF.value = (oRa.value * 1 - 459.67).toFixed(i);
 }
};

<h3>Temperature <input type="number" min="0" max="12" value="0" id="decimalTemp" name="decimal" placeholder="Decimal"> <small>Decimals<small></h3>
Celsius (oC) <input id="oC" type="number" min="-273.15" value="20">
<br> Kelvin (K) <input id="oK" type="number" min="0" value="293">
<br> Farenheit (oF) <input id="oF" type="number" min="-459.67" value="68">
<br> Rankine (oRa) <input id="oRa" type="number" min="0" value="528">

Complete code on JSFiddle

Answer 1

Keep it simple

Your code is overly complex as you are doing too many conversions C <=> F, C <=> Ra, and Ra <=> F can all be avoided.

When doing any type of unit conversion select a base unit and then only convert between the base unit and other units.

By using a base unit you can do all the conversions with only 6 formulas, rather than the 12 you are currently using.

General review

HTML

It is important for the content of a web page to have machine readable information that is unambiguous and semantically meaningful. Many people need assistance to interact with web pages, if the machine can not understand the content these people can not use the page.

Also bots (such as search engines use to rank pages) can not accurately assess a page's content if it does not understand what the page contains.

I am not HTML expert so these are just the basic points. See rewrite for more info

• Use appropriate elements

  • Use the Labels to label inputs
  • The different units temperatures are a list, use an unordered list to hold the inputs and labels.

• Related data

  Do NOT! use id to store semantic information. That does not mean you should not use meaningful id names, rather you should add the semantic information using more appropriate attributes and or elements.

  For example you use the event.target.id to determine which unit is being changed. Rather than use the id store the input elements unit type as a dataset attribute. See rewrite

JavaScript

• window

  Your use of window is redundant as window is the global this (scope).

  window.oninput = function(event) { is identical to oninput = function(event) {

• Add listeners don`t set listeners

  You should avoid assigning event listeners directly as they can be overwritten, by you accidentally, or by 3rd party content. E.G. adverts.

  Use addEventListener as it adds listeners

  E.G. oninput = function(event) { /*... code ... */ } should be addEventListener("input", function(event) { /*... code ... */ });

  NOTE: The event name when using addEventListener does not include the on prefix. "oninput" becomes "input"

• Constants

  For variables that do not change declare them as constants. eg const campo = event.target.id;

• Indentation

  Watch your indentation, the four lines below var i = document.ge... are incorrectly indented.

• Use standard keyboard characters

  The use of o via the keyboard dance LEFT ALT 167 is unnecessary as it adds nothing to the quality or readability of the code.

• Magic Numbers

  Numbers seldom have meaning on there own. They are also easy to get wrong it you have a lot of them. Give numbers names by defining them as constants.

  1.8 could mean anything... so remove the guess work with const RANKINE_SCALE = 1.8

Problem

Inputs can get stuck because the input step and min values are not set or incorrect

Changing the number of decimal points means you must change the step value for each input. E.G. Decimal 0 step 1, Decimal 1 step 0.1, 2 step 0.01, and so on.

To control the step position you can not set the min attribute to be a value that is not divisible by step. For all by Kelvin and Rankine the min value is problematic.

But using Kelvin as a the base unit we need only define its min attribute and in JS make sure its >= 0 so that the other units don't go out of bounds.

Rewrite

The rewrite add some CSS, a lot of HTML and splits the JS into many parts.

• Only the Kelvin input value is defined with a min and step and value. The other values are set when the JavaScript first runs by calling decimalEvent

• Each temperature input uses data-unit="" to name the unit used. This name is used to lookup the correct conversion function in the object toKelvin.

• Temperature and decimal input events are split. The decimal listener will call the Temperature listener which will use the default target kelvin if called without an event

const F_SCALE = 1.8, F_OFFSET = 459.67;
const C_OFFSET = 273.15;
const toKelvin = {
 Celsius: t => t + C_OFFSET,
 Farenheit: t => (t + F_OFFSET) / F_SCALE,
 Rankine: t => t / F_SCALE,
 Kelvin: t => t,
};
const fromKelvin = {
 Celsius: k => celsiusIn.value = round(k - C_OFFSET),
 Farenheit: k => farenheitIn.value = round(k * F_SCALE - F_OFFSET),
 Rankine: k => rankineIn.value = round(k * F_SCALE),
 Kelvin: k => kelvinIn.value = round(k),
};
const round = val => Math.round(val * round.scale) / round.scale;
decimalIn.addEventListener("input", decimalEvent);
temperatureList.addEventListener("input", temperatureEvent);
decimalEvent();
function decimalEvent() {
 const step = 1 / (round.scale = 10 ** decimalIn.value);
 celsiusIn.step = step;
 kelvinIn.step = step;
 farenheitIn.step = step;
 rankineIn.step = step;
 temperatureEvent();
} 
function temperatureEvent({target = kelvinIn} = {}) { 
 if (target.value !== "") {
 const unit = target.dataset.units;
 const kelvin = Math.max(0, toKelvin[unit](round(Number(target.value))));
 for (const convert of Object.values(fromKelvin)) { convert(kelvin) } 
 }
}

label {
 font-size: large;
}
input {
 width: 3em;
}
ul {
 width: 220px;
 display: flex;
 flex-direction: column;
 list-style-type: none;
 padding-inline-start: 0px;
}
li input {
 float: right;
 width: 7em;
}

<h3>Temperature unit converter</h3>
 <input type="number" min="0" max="5" value="0" step="1" id="decimalIn">
<label for="decimalIn"><small>Decimals<small></label>
<ul id="temperatureList">
<li>
 <label for="celsiusIn">Celsius (oC) </label>
 <input id="celsiusIn" type="number" data-units ="Celsius">
</li> 
<li> 
 <label for="kelvinIn">Kelvin (K) </label>
 <input id="kelvinIn" type="number" min="0" value="293" step="1" data-units="Kelvin">
</li> 
<li> 
 <label for="farenheitIn">Farenheit (oF) </label>
 <input id="farenheitIn" type="number" data-units="Farenheit">
</li> 
<li>
 <label for="rankineIn">Rankine (oRa) </label>
 <input id="rankineIn" type="number" data-units="Rankine"> 
</li> 
</ul>

Update

The original rewrite had a few problems.

• Could not type negative values in some situations
• Was converting digits past the decimal count
• Was displaying trailing zeros. Eg 0.000 should be just 0

The code has been changed to ignore partial inputs. E.G. the negative sign '-' before the numbers are added.

Using Math.round rather than Number.toFixed to set the number of decimal places.

Round the decimals before converting to base unit (kelvin) to stop fractions past the number of decimal making changes to other units.

• ARNON: I have never seen such beautiful code and I never imagined that it would be possible to make the code as simple as that. Your explanations made me understand JavaScript much more and for sure I will apply everything you taught.
• ARNON: I just realized now that the inputs are not accepting negative numbers directly from the keyboard.
• Blindman67: @ArnonRodrigues The inputs should be using the change event rather than the input event. However that would require a calculate button. You can also combine the two (events) and not set the value of the focused input element until it gets a change event
• Blindman67: @ArnonRodrigues I just noticed another problem. I am out of time ATM. Will update the answer addressing the problems tomorrow.
• ARNON: All right, @Blindman67! Your code was my basis for working with other measurement systems. The complete JavaScript was a little repetitive, but for me it's perfect!

Answer 2

The use of non-ASCII character o in variable names is surprising to me

---

There is a lot of repeated * 1 that is presumably used to convert string to number. parseFloat or parseInt will make intention more clear

---

There is a lot of repetition in formulas. I would get the new user-inputted value and convert to a standard format (for example, oC). I would then have one function to convert from oC to any of the other units

---

== should be replaced with === whenever possible

---

campo seems to be Portuguese for "field". Consensus is that coding should be in English

---

Minor bug: increasing number of decimals results in inaccuracies (It says 20oC is 293K, but should be 293.15K). Results need to be recalculated from last user-inputted value to be accurate

---

The default step for input tags is 1. This attribute should be changed to any to remove validation errors.

• ARNON: You helped me a lot! I really appreciate it! All the suggestions you made are now in use! Thank you very much!!

Answer 3

This code can be simplified a lot.

Don't use non alpha numeric characters for naming id's, variables, functions, etc.

You should consider changing the way you implemented the decimals. For example if you choose to have one decimal on a number like 2.19 you will get 2.1. I would expect that to be rounded to the nearest integer and that would be 2.2.

Apart from that you can definitely simplify the code quite a lot. Here is how I would do it:

// for being able to keep track of the unit we are using
// type Unit = "C" | "K" | "RA" | "RO" | "N" | "DE" | "RE"
// create a helper class that does the conversion
// it's easier if you have a base unit to convert to and from. I chose
// celsius
class Convertor {
 constructor(temp, unit) {
 this.tempInCelsius = this.convertToCelsius(temp, unit);
 }
 get toCelsius() {
 return this.tempInCelsius;
 }
 get toKelvin() {
 return this.tempInCelsius + 273.15;
 }
 get toFahrenheit() {
 return (this.tempInCelsius * 1.8) + 32;
 }
 get toRankine() {
 return (this.tempInCelsius * 9/5 + 491.67).toFixed(2)
 }
 get toRomer() {
 return this.tempInCelsius * 21/40 + 7.5;
 }
 get toNewton() {
 return this.tempInCelsius * 3.0303;
 }
 get toDelisle() {
 return (100 - this.tempInCelsius) * 3/2;
 }
 get toReaumur() {
 return this.tempInCelsius * 0.8;
 }
 convertToCelsius(temp, unit) {
 switch (unit) {
 case 'C':
 return temp;
 case 'K':
 return temp - 273.15;
 case 'RA':
 return (temp - 491.67) * 5/9;
 case 'RO':
 return (temp - 7.5) * 40/21;
 case 'N':
 return temp / 3.0303;
 case 'DE':
 return 100 - (temp * 2/3);
 case 'RE':
 return temp / 0.8;
 }
 }
}
window.oninput = function(event) {
 const campo = event.target.id; 
 const i = document.getElementById("decimalTemp").value;
 // you need to rename the id's of the inputs
 // the switch inside the convertToCelsius is using that id
 const degreesCelsius = new Convertor(+(document.getElementById(campo).value), campo);
 C.value = degreesCelsius.toCelsius;
 K.value = degreesCelsius.toKelvin;
 F.value = degreesCelsius.toFahrenheit;
 RA.value = degreesCelsius.toRankine;
 RO.value = degreesCelsius.toRomer;
 N.value = degreesCelsius.toNewton;
 DE.value = degreesCelsius.toDelisle;
 RE.value = degreesCelsius.toReaumur;
 };

In your html you rename your ID's like this:

<div class="block-input">
 <input id="C" type="number" min="-273.15" value="20">
 <h4>Celsius (degrees C)</h4>
 </div>
 <div class="block-input">
 <input id="K" type="number" min="0" value="293">
 <h4>Kelvin (K)</h4>
 </div>
 <div class="block-input">
 <input id="F" type="number" min="-459.67" value="68">
 <h4>Farenheit (degreesF)</h4>
 </div>
 <div class="block-input">
 <input id="RA" type="number" min="0" value="528">
 <h4>Rankine (degreesRa)</h4>
 </div>
 <div class="block-input">
 <input id="RO" type="number" min="-135.90" value="18">
 <h4>Rømer (degreesRø)</h4>
 </div>
 <div class="block-input">
 <input id="N" type="number" min="-90.14" value="7">
 <h4>Newton (degreesN)</h4>
 </div>
 <div class="block-input">
 <input id="DE" type="number" min="-559.73" value="120">
 <h4>Delisle (degreesDe)</h4>
 </div>
 <div class="block-input">
 <input id="RE" type="number" min="-218.52" value="16">
 <h4>Reaumur (degreesRe)</h4>
 </div>

Here is a jsfiddle: https://jsfiddle.net/4f5nsyo8/1/

• ARNON: I believe the Constructor will save many lines of code. Mainly when I need to write other measure systems. But how would do to keep the decimals working? Do you consider Constructor the best place to put it on?