2
\$\begingroup\$

I've made a simple console application using JavaScript, NodeJS and yargs.

index.js

This is the startup file.

#!/usr/bin/env node
'use strict';

const yargs = require('yargs');
const argv = yargs
.option('celsius', {alias: 'c', default: false})
.option('fahrenheit', {alias: 'f', default: false})
.option('kelvin', {alias: 'k', default: false})
.help('h')
.argv;

let value = argv._[0];

if (!value) {
  console.log('You need to pass a value to be converted.');
} else {
  const convert = require('./convert')

  if (argv.celsius) {
    console.log(convert.fromCelsius(value));
  } else if (argv.fahrenheit) {
    console.log(convert.fromFahrenheit(value));
  } else if (argv.kelvin) {
    console.log(convert.fromKelvin(value));
  } else {
    console.log("You need to pass a valid flag.")
  }
}

convert.js

This is required by index.js in order to transform values. Each scale has it own file.

'use strict';

let fixFloat = (value) => parseFloat(value).toFixed(2);

let fromCelsius = (value) => {
  const conversion = require('./celsius');
  let kelvin = fixFloat(conversion.toKelvin(value));
  let fahrenheit = fixFloat(conversion.toFahrenheit(value));
  return `${value}°C = ${fahrenheit}°F = ${kelvin}K`;
};

let fromFahrenheit = (value) => {
  const conversion = require('./fahrenheit');
  let celsius = fixFloat(conversion.toCelsius(value));
  let kelvin = fixFloat(conversion.toKelvin(value));
  return `${value}°F = ${celsius}°C = ${kelvin}K`;
};

let fromKelvin = (value) => {
  const conversion = require('./kelvin');
  let celsius = fixFloat(conversion.toCelsius(value));
  let fahrenheit = fixFloat(conversion.toFahrenheit(value));
  return `${value}K = ${celsius}°C = ${fahrenheit}°F`;
};

module.exports = {
  fromCelsius,
  fromFahrenheit,
  fromKelvin,
};

celsius.js

'use strict';

let toKelvin = (value) => value + 273.15;
let toFahrenheit = (value) => (value * (9 / 5)) + 32;

module.exports = {
  toKelvin,
  toFahrenheit,
};

fahrenheit.js

'use strict';

let toCelsius = (value) => (value - 32) * (5 / 9);
let toKelvin = (value) => (value + 459.67) * (5 / 9);

module.exports = {
  toCelsius,
  toKelvin,
};

kelvin.js

'use strict';

let toCelsius = (value) => value - 273.15;
let toFahrenheit = (value) => 1.8 * toCelsius(value) + 32;

module.exports = {
  toCelsius,
  toFahrenheit,
};

You can see it running by installing it via npm

npm i -g thermo.js
thermo 10 -celsius
=> 10°C = 50.00°F = 283.15K

Is my implementation good? This is my first non-web JavaScript program. I want to make the code more idiomatic. I think I messed up some of ES6 variables declaration.

| improve this question | | | | |
\$\endgroup\$
3
\$\begingroup\$

Generalizing code to write less of it

Adding support for a new time scale

To test your code for maintainability let's go through a hypothetical situation. What if a new temperature scale were introduced - say Felsius?

To add support for the up-and-coming scale you would need to add a new file, felsius.js, which will contain 3 conversion functions: toKelvin toCelsius toFahrenheit. Next add toFelsius functions in each of the existing temperature scale files.

Alright, lots of maths... but awesome. We did it! Now simply to add:

let fromFelsius = (value) => {
  const conversion = require('./felsius');
  let celsius = fixFloat(conversion.toCelsius(value));
  let fahrenheit = fixFloat(conversion.toFahrenheit(value));
  let kelvin = fixFloat(conversion.toKelvin(value));
  return `${value}°ϵ = ${celsius}°F = ${fahrenheit}°F = ${kelvin}K`;
};

And add:

let felsius = fixFloat(conversion.toFelsius(value));
`${felsius}°ϵ`;

To each fromX function.

Finally done! Let's reflect:

Hmmmm... That seemed like an awful lot of work - possibly prone to errors too.

Unfortunately we don't have time for any more reflection because the project manager just informed us of a huge user base that firmly believes in the Rankine scale.

Possible modification

Having a specialized conversion function between each temperature scale requires \$n^2\$ conversion functions (where \$n\$ is the number of temperature scales you support). You could get by with \$2n\$ if you used a standard temperature scale as the root of all of your conversions. For example fahrenheit.js using Celsius as the standard:

'use strict';

let fromCelsius = (value) => (value * (9 / 5)) + 32;
let toCelsius = (value) => (value - 32) * (5 / 9);
let toString = (value) => `{value}°F`

module.exports = {
  fromCelsius,
  toCelsius,
  toString,
};

Now convert.js

'use strict';

let fixFloat = (value) => parseFloat(value).toFixed(2);

const temperatureScales = [
  {
    name: 'Fahrenheit',
    file: './farhenheit.js'
  },
  {
    name: 'Celsius',
    file: './celsius.js'
  },
  {
    name: 'Kelvin',
    file: './kelvin.js'
  },
  {
    name: 'Felsius',
    file: './felsius.js'
  },
];

// add conversion code for each temperature scale
module.exports = temperatureScales.reduce((m, fromScale) => {
  const fromTemp = require(fromScale.file);

  // generate the `fromFooBar` function
  m['from' + fromScale.name] = (value) => {
    const standard = fromTemp.toCelsius(value);

    let str = fromTemp.toString(fixFloat(value));

    // generate the rest of the result string
    return temperatureScales.reduce((str, toScale) => {
      if (toScale == fromScale) { // don't recompute the initial temperature
        return str;
      }

      const toTemp = require(toScale.file);
      const temperatureValue = fixFloat(toTemp.fromCelsius(standard));
      const temperatureString = toTemp.toString(temperatureValue);
      return str + " = " + temperatureString;
    }, str);
  };

  return m;
}, module.exports);

Eliminating some of the redundancies makes it much easier to add Rankine support. Simply add { name: 'Rankine', file: './rankine.js' } to the temperatureScales list. Then add three functions: toCelsius fromCelsius toString to ./rankine.js.

Often redundancies in code lead to more work when maintaining. However, the generalized code might be less readable than the redundant version (nested reduce functions - why would anyone do that??).

Nice work overall! For your purpose the redundancies are probably fine -- just some food for thought.

| improve this answer | | | | |
\$\endgroup\$
2
\$\begingroup\$

I think I messed up some of ES6 variables declaration.

Well I don't know if it is really "messed up" but you could consider using const for any variable that doesn't get reassigned within the current scope. For instance, index.js has the line below:

let value = argv._[0];

I don't see any subsequent lines in that file that re-assign a value to that variable. So it could be changed to

const value = argv._[0];

The same is true for the code in other files, like in convert.js:

let kelvin = fixFloat(conversion.toKelvin(value));
let fahrenheit = fixFloat(conversion.toFahrenheit(value));

Those variables also don't appear to be re-assigned, so they could be declared as constants.

| improve this answer | | | | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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