# Celsius and Fahrenheit Converter in JavaScript

Posted on

Problem

I am learning JavaScript. Please review my code. Just created this Celsius & Fahrenheit Converter. Any suggestions to improve my code will be really helpful.

``````const celsiusToFahrenheit = document.getElementById('celsiusToFarhenhite');
const fahrenheitToCelsius = document.getElementById('farhenhiteToCelsius');
const converterButton = document.querySelector('.calculate');

event.preventDefault();

const celsiusToFahrenheitValue = (celsiusToFahrenheit.value * 9/5 + 32);
const fahrenheitToCelsiusValue = (fahrenheitToCelsius.value - 32) * 5/9;

if(celsiusToFahrenheit.value){
document.write(`<h1>Celcius To Fahrenheit Result: </h1> \${celsiusToFahrenheitValue} &#8457;`);
}else if (fahrenheitToCelsius.value){
document.write(`<h1>Fahrenheit To Celcius Result: </h1> \${fahrenheitToCelsiusValue} &#8451;`);
}else{
document.write(`<h1>You must enter a number!</h1>`);
}
})
``````

Solution

Use proper spelling – it’ll looks more professional and can prevent bugs stemming from the typo. `Farhenhite` should be `Fahrenheit`, and `Celcius` should be `Celsius`.

Use proper indentation – every new block should open a new indentation level. You’re doing this well with the `if`/`else`s, but you should also do it for the whole click listener as well:

``````converterButton.addEventListener('click', function(event){
event.preventDefault();

const celsiusToFahrenheitValue = (celsiusToFahrenheit.value * 9/5 + 32);
const fahrenheitToCelsiusValue = (fahrenheitToCelsius.value - 32) * 5/9;
// etc
``````

`preventDefault`? Is the button inside a `<form>`? If so, the `preventDefault` is fine. Otherwise, it won’t do anything and can be removed.

Avoid `document.write` – it can be insecure and somewhat confusing. Here, since it’s called after the page loads, it will replace the whole document with the new HTML, which is probably not what you want – you’d probably rather preserve the converter and let people continue to convert after pressing the button once. Instead, populate, say, a results element with the results.

Confusing variable names Without reading the variable definition, it isn’t entirely clear what the difference between `celsiusToFahrenheitValue` and `celsiusToFahrenheit.value` is. Consider only calculating the new value when it’s needed, inside the `if` statements, and calling it something like `convertedToFah`. Also consider renaming `celsiusToFahrenheit` to just plain `celsius`, since the value it will hold is the plain celcius value.

``````<h1 class='conversion-type'></h1>
<div class='result'></div>
``````
``````if (celsius.value) {
const convertedToFah = (celsius.value * 9/5 + 32);
document.querySelector('.conversion-type').textContent = 'Celsius To Fahrenheit Result:';
document.querySelector('.result').textContent = `\${convertedToFah} ℉`;
}
``````

When one input is changed, consider clearing the other input to make it clear what will be converted when the button is pressed – use an `input` listener.

I think you should write your code to more clearly treat nouns as nouns and verbs as verbs. `celsiusToFahrenheit` is a noun, but its name makes it sound like a verb. Getting rid of the to part makes your variable names simpler, and more clearly nouns.

The line `celsiusToFahrenheitValue = (celsiusToFahrenheit.value * 9/5 + 32);` is a verb, but you’re doing it in a `const` declaration, making it seem like a noun. In addition, you only need to calculate one. Moving the declaration into if-branches both saves calculation, makes it clearer that you’re doing a calculation, and allows you to not introduce another const.

Your if-then logic catches the case where neither are set, but doesn’t anticipate the possibility that both are set. You should either disable one entry when the other is entered, or have logic dealing with this case. You could also separate out the two converter: have an input box for Celcius, a “convert from Celcius to Farenheit” button, and an output box for the calculation, and then another input box for Farenheit, “convert from Farenheit to Celcius” button, and another output box for that output.

Some of your lines are almost 100 characters long. If a language uses semicolons to demarcate commands, that implies that it ignores carriage returns. So if a line is getting long, you can just put a line break in.

Javascript isn’t my primary language, so hopefully I have the syntax correct on this:

``````const Celcius = document.getElementById('celciusToFarhenheit');
const Fahrenheit = document.getElementById('farhenheitToCelcius');
const converterButton = document.querySelector('.calculate');

function(event){
event.preventDefault();
if(Celcius.value && !Farenheit.value){
document.write(`<h1>Celcius To Fahrenheit Result: </h1>
\${Celcius.value * 9/5 + 32 } &#8457;`);
}
if(Farenheit.value && !Celcius.value){
document.write(`<h1>Celcius To Fahrenheit Result: </h1>
\${(Fahrenheit.value - 32) * 5/9} &#8451;`);
}
if (Celcius.value && Farenheit.value){
document.write(`<h1>Enter only one number.</h1>`);
}
if !(Celcius.value || Farenheit.value){
document.write(`<h1>You must enter a number!</h1>`);
}
}
)
``````