CODEWITHSUNDEEP
javascriptfunction
Valy
Valy

Reputation: 49

Could someone please tell me how I could simplify this code?

In practice I have this code and it seems very tiring to do all those if there is a way to make it easier? But that it works correctly? Because this code not only has * (multiplications) it also has 3 / (divisions). If it only had multiplications it was easy to do it but it also has those divisons so it becomes more difficult to do so. If anyone could help me thanks.

        function calculateLength() {
        
        
        
            var lengthinput = parseFloat(document.getElementById('lengthinput').value);


            var oper = document.getElementById('lengthselector1').value;
            var oper2 = document.getElementById('lengthselector2').value;


            if(oper === 'k' && oper2 === 'm')
            {
                document.getElementById('lengthresult').value = lengthinput*1000 || 0;
            }

            
            if(oper === 'k' && oper2 === 'd')
            {
                document.getElementById('lengthresult').value = lengthinput*10000 || 0;
            }

            if(oper === 'k' && oper2 === 'c')
            {
                document.getElementById('lengthresult').value = lengthinput*100000 || 0;
            }
            
            if(oper === 'k' && oper2 === 'mi')
            {
                document.getElementById('lengthresult').value = lengthinput*1000000 || 0;
            }

            if(oper === 'k' && oper2 === 'mic')
            {
                document.getElementById('lengthresult').value = lengthinput*1.0000E+9 || 0;
            }               
            
            if(oper === 'k' && oper2 === 'de')
            {
                document.getElementById('lengthresult').value = lengthinput*100 || 0;
            }

            if(oper === 'k' && oper2 === 'h')
            {
                document.getElementById('lengthresult').value = lengthinput*10 || 0;
            }   

            if(oper === 'k' && oper2 === 'me')
            {
                document.getElementById('lengthresult').value = lengthinput/1000 || 0;
            }
            
            if(oper === 'k' && oper2 === 'g')
            {
                document.getElementById('lengthresult').value = lengthinput/1000000 || 0;
            }

            if(oper === 'k' && oper2 === 'z')
            {
                document.getElementById('lengthresult').value = lengthinput/1.0000E+18 || 0;
            }               
            
            if(oper === 'k' && oper2 === 'i')
            {
                document.getElementById('lengthresult').value = lengthinput*39370.0787 || 0;
            }

            if(oper === 'k' && oper2 === 'a')
            {
                document.getElementById('lengthresult').value = lengthinput*1.0000E+13 || 0;
            }           
                            

        }

Upvotes: 0

Views: 86

Answers (4)

trincot
trincot

Reputation: 351403

All your if conditions test for oper==='k', so that can become just one if statement around all the rest. For oper2 make a lookup table:

function calculateLength() {
    var lengthinput = parseFloat(document.getElementById('lengthinput').value);
    var oper = document.getElementById('lengthselector1').value;
    if (oper === 'k') {
        var oper2 = document.getElementById('lengthselector2').value;
        var coefficient = {
            'i': 39370.0787,
            'z': 1e-18,
            'g': 1e-6,
            'me': 1e-3,
            'h': 10,
            'de': 100,
            'm': 1e3,
            'd': 1e4,
            'c': 1e5,
            'mi': 1e6,
            'mic': 1e9,
            'a': 1e13
        }[oper2]; // <-- here we perform the lookup
        if (coefficient) {
            document.getElementById('lengthresult').value = lengthinput*coefficient || 0;
        }
    }
}

Note that 1.0000E+9 in your code is equal to 1e9. The additional decimal zeroes do not change anything, nor does the + in the exponent part.

In some cases your code uses the standard notation, like 1000000, which can also be written in scientific notation: 1e6. I would try to use the same notation for all powers of 10, as then you can more clearly see how the different coefficients compare.

Also, a division by a power of 10 is the same as a multiplication by a power of ten where the exponent is negated: For example:

length/1e18 === length*1e-18

That means you can write all those coefficients as multipliers, and in scientific notation (except for the irregular 39370.0787 where you can opt to keep the standard notation).

Just to compare, you can write that lookup table ("mapping object") without scientific notation as follows:

    var coefficient = {
        'i': 39370.0787,
        'z': 0.000000000000000001,
        'g': 0.000001,
        'me': 0.001,
        'h': 10,
        'de': 100,
        'm': 1000,
        'd': 10000,
        'c': 100000,
        'mi': 1000000,
        'mic': 1000000000,
        'a': 10000000000000
    }[oper2]; // <-- here we perform the lookup

Upvotes: 5

Thomas Sablik
Thomas Sablik

Reputation: 16448

You could use a lookup table for the coefficient, e.g.

const coefficient = { k: { mic: 1.0000E+9, i: 39370.0787 } };

and use it with

document.getElementById('lengthresult').value = lengthinput*coefficient[oper][oper2] || 0;

Example:

function calculateLength() {
    const coefficient = { k: { mic: 1.0000E+9, i: 39370.0787 } };
    const lengthinput = parseFloat(document.getElementById('lengthinput').value);


    const oper = document.getElementById('lengthselector1').value;
    const oper2 = document.getElementById('lengthselector2').value;

    if (oper in coefficient && oper2 in coefficient[oper]) {
        document.getElementById('lengthresult').value = lengthinput*coefficient[oper][oper2] || 0;
    }    
}

That way you can easily extend it:

function calculateLength() {
    const coefficient = {
        k: { mic: 1.0000E+9, i: 39370.0787 },
        l: { abc: 123, def: 567 }
    };
    const lengthinput = parseFloat(document.getElementById('lengthinput').value);


    const oper = document.getElementById('lengthselector1').value;
    const oper2 = document.getElementById('lengthselector2').value;

    if (oper in coefficient && oper2 in coefficient[oper]) {
        document.getElementById('lengthresult').value = lengthinput*coefficient[oper][oper2] || 0;
    }    
}

Upvotes: 4

user1751825
user1751825

Reputation: 4309

Just another slight variation. I wrote this before I saw the other answers, so thought I might as well include it...

function calculateLength() {
    var lengthinput = parseFloat(document.getElementById('lengthinput').value);

    var oper = document.getElementById('lengthselector1').value;
    var oper2 = document.getElementById('lengthselector2').value;

    if (oper === 'k') {
        document.getElementById('lengthresult').value = (function () {
            switch (oper2) {
                case 'm':
                    return lengthinput * 1000 || 0;
                case 'd':
                    return lengthinput * 10000 || 0;
                case 'c':
                    return lengthinput * 100000 || 0;
                case 'mi':
                    return lengthinput * 1000000 || 0;
                case 'mic':
                    return lengthinput * 1.0000E+9 || 0;
                case 'de':
                    return lengthinput * 100 || 0;
                case 'h':
                    return lengthinput * 10 || 0;
                case 'me':
                    return lengthinput / 1000 || 0;
                case 'g':
                    return lengthinput / 1000000 || 0;
                case 'z':
                    return lengthinput / 1.0000E+18 || 0;
                case 'i':
                    return lengthinput * 39370.0787 || 0;
                case 'a':
                    return lengthinput * 1.0000E+13 || 0;
            }
        })();
    }
}

Upvotes: 0

Ali Esmailpor
Ali Esmailpor

Reputation: 1201

You can try this:

function calculateLength() {
  const data = [
    {
      oper1: "k",
      oper2: "m",
      time: 1000,
    },
    {
      oper1: "k",
      oper2: "d",
      time: 10000,
    },
  ];
  //---
  var lengthinput = parseFloat(document.getElementById("lengthinput").value);
  //---
  var oper = document.getElementById("lengthselector1").value;
  var oper2 = document.getElementById("lengthselector2").value;
  //---
  for (var count = 0; count < data.length; count++) {
    if (data[count].oper1 === oper && data[count].oper2 === oper2) {
      document.getElementById("lengthresult").value =
        lengthinput * data[count].time || 0;
    }
  }
}

Upvotes: 1

Related Questions