How would I do the same thing with less code?

Viewed 83

I am making a unit converter - but my code looks very messy and I am a beginner so I don't know any way to make it more efficient. How would I do make the below code do the same thing but with more logic and less code?

let x = document.getElementById("unitAmount");
function convert() {
    const fromUnit = document.getElementById('fromUnitSelector').value;
    const toUnit = document.getElementById('toUnitSelector').value;
    const output = document.getElementById('outputBox');
    
    if(fromUnit == 1 && toUnit == 1) { output.value = x; }
    if(fromUnit == 1 && toUnit == 2) { output.value = (x * 10.7639).toFixed(2); }
    if(fromUnit == 1 && toUnit == 3) { output.value = (x / 508.74).toFixed(4); }
    if(fromUnit == 1 && toUnit == 4) { output.value = (x / 31.79).toFixed(4); }
    if(fromUnit == 1 && toUnit == 5) { output.value = (x / 16.9).toFixed(4); }
    if(fromUnit == 1 && toUnit == 6) { output.value = (x / 1.985).toFixed(4); }
    if(fromUnit == 2 && toUnit == 1) { output.value = (x / 21.39).toFixed(4); }
    if(fromUnit == 2 && toUnit == 2) { output.value = x; }
    if(fromUnit == 2 && toUnit == 3) { output.value = (x / 5476).toFixed(4); }
    if(fromUnit == 2 && toUnit == 4) { output.value = (x / 342.25).toFixed(4); }
    if(fromUnit == 2 && toUnit == 5) { output.value = (x / 85.56).toFixed(4); }
    if(fromUnit == 2 && toUnit == 6) { output.value = (x / 21.39).toFixed(4); }
    if(fromUnit == 3 && toUnit == 1) { output.value = (x * 508.74).toFixed(2); }
    if(fromUnit == 3 && toUnit == 2) { output.value = (x * 5476).toFixed(2); }
    if(fromUnit == 3 && toUnit == 3) { output.value = x; }
    if(fromUnit == 3 && toUnit == 4) { output.value = (x * 16).toFixed(2); }
    if(fromUnit == 3 && toUnit == 5) { output.value = (x * 64).toFixed(2); }
    if(fromUnit == 3 && toUnit == 6) { output.value = (x * 256).toFixed(2); }
    if(fromUnit == 4 && toUnit == 1) { output.value = (x * 31.79).toFixed(2); }
    if(fromUnit == 4 && toUnit == 2) { output.value = (x * 342.25).toFixed(2); }
    if(fromUnit == 4 && toUnit == 3) { output.value = (x / 16).toFixed(4); }
    if(fromUnit == 4 && toUnit == 4) { output.value = x; }
    if(fromUnit == 4 && toUnit == 5) { output.value = (x * 4).toFixed(2); }
    if(fromUnit == 4 && toUnit == 6) { output.value = (x * 16).toFixed(2); }
    if(fromUnit == 5 && toUnit == 1) { output.value = (x * 7.94).toFixed(2); }
    if(fromUnit == 5 && toUnit == 2) { output.value = (x * 85.56).toFixed(2); }
    if(fromUnit == 5 && toUnit == 3) { output.value = (x / 64).toFixed(4); }
    if(fromUnit == 5 && toUnit == 4) { output.value = (x / 4).toFixed(4); }
    if(fromUnit == 5 && toUnit == 5) { output.value = x; }
    if(fromUnit == 5 && toUnit == 6) { output.value = (x * 4).toFixed(2); }
    if(fromUnit == 6 && toUnit == 1) { output.value = (x * 1.985).toFixed(2); }
    if(fromUnit == 6 && toUnit == 2) { output.value = (x * 21.39).toFixed(2); }
    if(fromUnit == 6 && toUnit == 3) { output.value = (x / 256).toFixed(4); }
    if(fromUnit == 6 && toUnit == 4) { output.value = (x / 16).toFixed(4); }
    if(fromUnit == 6 && toUnit == 5) { output.value = (x / 4).toFixed(4); }
    if(fromUnit == 6 && toUnit == 6) { output.value = x; }
}
3 Answers

You can make a multi-dimensional array that removes much of the redundancy:

function convert() {
    let x = document.getElementById("unitAmount").value;
    const fromUnit = document.getElementById('fromUnitSelector').value;
    const toUnit = document.getElementById('toUnitSelector').value;
    const output = document.getElementById('outputBox'); 

    a = [
        // Each element consists of a value, optionally followed by a toFixed value:
        [
            [ x ], // toUnit == 1
            [ (x * 10.7639), 2 ],    // fromUnit == 1 && toUnit == 2
            [ (x / 508.74), 4 ],     // fromUnit == 1 && toUnit == 3
            [ (x / 31.79), 4 ],      // fromUnit == 1 && toUnit == 4
            [ (x / 16.9), 4 ],       // fromUnit == 1 && toUnit == 5
            [ (x / 1.985), 4 ],      // fromUnit == 1 && toUnit == 6
        ],

        [
            [ (x / 21.39), 4 ],      // fromUnit == 2 && toUnit == 1
            [ x ],                   // fromUnit == 2 && toUnit == 2
            [ (x / 5476), 4 ],       // fromUnit == 2 && toUnit == 3
            [ (x / 342.25), 4 ],     // fromUnit == 2 && toUnit == 4
            [ (x / 85.56), 4 ],      // fromUnit == 2 && toUnit == 5
            [ (x / 21.39), 4 ],      // fromUnit == 2 && toUnit == 6
        ],

        [
            [ (x * 508.74), 2 ],  // fromUnit == 3 && toUnit == 1
            [ (x * 5476), 2 ],    // fromUnit == 3 && toUnit == 2
            [ x ],                // fromUnit == 3 && toUnit == 3
            [ (x * 16), 2 ],      // fromUnit == 3 && toUnit == 4
            [ (x * 64), 2 ],      // fromUnit == 3 && toUnit == 5
            [ (x * 256), 2 ],     // fromUnit == 3 && toUnit == 6
        ],

        [
            [ (x * 31.79), 2 ],   // fromUnit == 4 && toUnit == 1
            [ (x * 342.25), 2 ],  // fromUnit == 4 && toUnit == 2
            [ (x / 16), 4 ],      // fromUnit == 4 && toUnit == 3
            [ x ],                // fromUnit == 4 && toUnit == 4
            [ (x * 4), 2 ],       // fromUnit == 4 && toUnit == 5
            [ (x * 16), 2 ],      // fromUnit == 4 && toUnit == 6
        ],

        [
            [ (x * 7.94), 2 ],    // fromUnit == 5 && toUnit == 1
            [ (x * 85.56), 2 ],   // fromUnit == 5 && toUnit == 2
            [ (x / 64), 4 ],      // fromUnit == 5 && toUnit == 3
            [ (x / 4), 4 ],       // fromUnit == 5 && toUnit == 4
            [ x ],                // fromUnit == 5 && toUnit == 5
            [ (x * 4), 2 ],       // fromUnit == 5 && toUnit == 6
        ],

        [
            [ (x * 1.985), 2 ],   // fromUnit == 6 && toUnit == 1
            [ (x * 21.39), 2 ],   // fromUnit == 6 && toUnit == 2
            [ (x / 256), 4 ],     // fromUnit == 6 && toUnit == 3
            [ (x / 16), 4 ],      // fromUnit == 6 && toUnit == 4
            [ (x / 4), 4 ],       // fromUnit == 6 && toUnit == 5
            [ x ],                // fromUnit == 6 && toUnit == 6
        ],
    ]

    let elmt = a[ fromUnit - 1 ][ toUnit - 1 ];
    let val = elmt[ 0 ]; // Get the value
    if ( elmt[ 1 ] ) { // If it should be toFixed():
        val = val.toFixed( elmt[ 1 ] );
    }
    output.value = val;
}

Looking at patterns in the conversion, I believe you could massively shorten the code by using a lookup map like below.

It works like this:

  • If fromUnit === toUnit just return x.

  • For all cases where fromUnit < toUnit you look up conversionMap[fromUnit][toUnit]. And the result is a two element array with the conversion factor and either a 1 for multiply or a -1 for divide.

  • For all cases where toUnit < fromUnit you look up conversionMap[toUnit][fromUnit], using the same conversion factor, but the opposite operation, i.e. 1 for divide, -1 for multiply.

  • Your pattern indicates to use .toFixed(4) when dividing and .toFixed(2) when multiplying, so there's no need to store that additional information.

Here's a CodeSandbox:

Edit currying-frost-udp0c

const conversionMap = {
  1: {
    2: [10.7639, 1],
    3: [508.74, -1],
    4: [31.79, -1],
    5: [16.9, -1],
    6: [1.985, -1],
  },
  2: {
    3: [5476, -1],
    4: [342.25, -1],
    5: [85.56, -1],
    6: [21.39, -1],
  },
  3: {
    4: [16, 1],
    5: [64, 1],
    6: [256, 1],
  },
  4: {
    5: [4, 1],
    6: [16, 1],
  },
  5: {
    6: [4, 1],
  },
};

function handleConversion(x, from, to) {
  if (from === to) return x;
  if (from < to) {
    const [factor, power] = conversionMap[from][to];
    x *= Math.pow(factor, power);
    if (power === 1) return x.toFixed(2);
    return x.toFixed(4);
  } else {
    const [factor, power] = conversionMap[to][from];
    x *= Math.pow(factor, power * -1);
    if (power === 1) return x.toFixed(4);
    return x.toFixed(2);
  }
}

$(function() {
  $("input").on("change", changeHandler);
  $("select").on("change", changeHandler);

  function changeHandler() {
    const unitAmount = $("#unitAmount").val();
    const fromUnit = $("#fromUnitSelector").val();
    const toUnit = $("#toUnitSelector").val();

    // using the original convert function
    $("#outputBox1").text(convert(unitAmount, fromUnit, toUnit));
    // using the shorter version
    $("#outputBox2").text(handleConversion(unitAmount, fromUnit, toUnit));
  }
});

It's not possible to simplify your code without sacrificing much of what little readability it has.


UPDATE: I seem to have been proven wrong by other answers.


But that didn't stop me from trying to use a pairing function to map each [from, to] pair to a single key in a map of conversion functions.

You can compare the results of your original function with mine below, to confirm they produce the same output for the same input.

const cantor = (n, y) => ((n + y) * (n + y + 1)) / 2 + y;

const convert = (x, from, to) => ({
  '4': n => n,
  '8': n => (n * 10.7639).toFixed(2),
  '13': n => (n / 508.74).toFixed(4),
  '19': n => (n / 31.79).toFixed(4),
  '26': n => (n / 16.9).toFixed(4),
  '34': n => (n / 1.985).toFixed(4),
  '7': n => (n / 21.39).toFixed(4),
  '12': n => n,
  '18': n => (n / 5476).toFixed(4),
  '25': n => (n / 342.25).toFixed(4),
  '33': n => (n / 85.56).toFixed(4),
  '42': n => (n / 21.39).toFixed(4),
  '11': n => (n * 508.74).toFixed(2),
  '17': n => (n * 5476).toFixed(2),
  '24': n => n,
  '32': n => (n * 16).toFixed(2),
  '41': n => (n * 64).toFixed(2),
  '51': n => (n * 256).toFixed(2),
  '16': n => (n * 31.79).toFixed(2),
  '23': n => (n * 342.25).toFixed(2),
  '31': n => (n / 16).toFixed(4),
  '40': n => n,
  '50': n => (n * 4).toFixed(2),
  '61': n => (n * 16).toFixed(2),
  '22': n => (n * 7.94).toFixed(2),
  '30': n => (n * 85.56).toFixed(2),
  '39': n => (n / 64).toFixed(4),
  '49': n => (n / 4).toFixed(4),
  '60': n => n,
  '72': n => (n * 4).toFixed(2),
  '29': n => (n * 1.985).toFixed(2),
  '38': n => (n * 21.39).toFixed(2),
  '48': n => (n / 256).toFixed(4),
  '59': n => (n / 16).toFixed(4),
  '71': n => (n / 4).toFixed(4),
  '84': n => n
}[cantor(from, to)](x));

for (let from = 1; from <= 6; from++) {
  for (let to = 1; to <= 6; to++) {
    console.log({ n: 1337, from, to, result: convert(1337, from, to) });
  }
}

function convert(x, from, to) {
  if (from == 1 && to == 1) { return x; }
  if (from == 1 && to == 2) { return (x * 10.7639).toFixed(2); }
  if (from == 1 && to == 3) { return (x / 508.74).toFixed(4); }
  if (from == 1 && to == 4) { return (x / 31.79).toFixed(4); }
  if (from == 1 && to == 5) { return (x / 16.9).toFixed(4); }
  if (from == 1 && to == 6) { return (x / 1.985).toFixed(4); }
  if (from == 2 && to == 1) { return (x / 21.39).toFixed(4); }
  if (from == 2 && to == 2) { return x; }
  if (from == 2 && to == 3) { return (x / 5476).toFixed(4); }
  if (from == 2 && to == 4) { return (x / 342.25).toFixed(4); }
  if (from == 2 && to == 5) { return (x / 85.56).toFixed(4); }
  if (from == 2 && to == 6) { return (x / 21.39).toFixed(4); }
  if (from == 3 && to == 1) { return (x * 508.74).toFixed(2); }
  if (from == 3 && to == 2) { return (x * 5476).toFixed(2); }
  if (from == 3 && to == 3) { return x; }
  if (from == 3 && to == 4) { return (x * 16).toFixed(2); }
  if (from == 3 && to == 5) { return (x * 64).toFixed(2); }
  if (from == 3 && to == 6) { return (x * 256).toFixed(2); }
  if (from == 4 && to == 1) { return (x * 31.79).toFixed(2); }
  if (from == 4 && to == 2) { return (x * 342.25).toFixed(2); }
  if (from == 4 && to == 3) { return (x / 16).toFixed(4); }
  if (from == 4 && to == 4) { return x; }
  if (from == 4 && to == 5) { return (x * 4).toFixed(2); }
  if (from == 4 && to == 6) { return (x * 16).toFixed(2); }
  if (from == 5 && to == 1) { return (x * 7.94).toFixed(2); }
  if (from == 5 && to == 2) { return (x * 85.56).toFixed(2); }
  if (from == 5 && to == 3) { return (x / 64).toFixed(4); }
  if (from == 5 && to == 4) { return (x / 4).toFixed(4); }
  if (from == 5 && to == 5) { return x; }
  if (from == 5 && to == 6) { return (x * 4).toFixed(2); }
  if (from == 6 && to == 1) { return (x * 1.985).toFixed(2); }
  if (from == 6 && to == 2) { return (x * 21.39).toFixed(2); }
  if (from == 6 && to == 3) { return (x / 256).toFixed(4); }
  if (from == 6 && to == 4) { return (x / 16).toFixed(4); }
  if (from == 6 && to == 5) { return (x / 4).toFixed(4); }
  if (from == 6 && to == 6) { return x; }
}

for (let from = 1; from <= 6; from++) {
  for (let to = 1; to <= 6; to++) {
    console.log({ n: 1337, from, to, result: convert(1337, from, to) });
  }
}

Related