CODEWITHSUNDEEP
javascriptfunction
Slick
Slick

Reputation: 47

How do I make these functions shorter

I was making a simple calculator with two input boxes for each number, and four buttons for each operation underneath. After pressing one of the buttons (Ex:Add) They would perform the operation. However, inside each function I had to keep writing these two lines:

var Value1 = document.getElementById('Value1').valueAsNumber;
var Value2 = document.getElementById('Value2').valueAsNumber;

Is there a way I could write these before I make each individual function? Could someone with more skill possibly show me what I could do?

function add() {
    var Value1 = document.getElementById('Value1').valueAsNumber;
    var Value2 = document.getElementById('Value2').valueAsNumber;
    var Total = Value1 + Value2;
    document.getElementById('demo').innerHTML = "Total: " + Total;
}
function sub() {
    var Value1 = document.getElementById('Value1').valueAsNumber;
    var Value2 = document.getElementById('Value2').valueAsNumber;
    var Total = Value1 - Value2;
    document.getElementById('demo').innerHTML = "Total: " + Total;
}
function mul() {
    var Value1 = document.getElementById('Value1').valueAsNumber;
    var Value2 = document.getElementById('Value2').valueAsNumber;
    var Total = Value1 * Value2;
    document.getElementById('demo').innerHTML = "Total: " + Total;
}
function div() {
    var Value1 = document.getElementById('Value1').valueAsNumber;
    var Value2 = document.getElementById('Value2').valueAsNumber;
    var Total = Value1 / Value2;
    document.getElementById('demo').innerHTML = "Total: " + Total;
}
First Number: <input type="number" id="Value1"><br><br>
Second Number: <input type="number" id="Value2"><br><br>

<button type="button" onclick="add()">Add</button>
<button type="button" onclick="sub()">Subtract</button>
<button type="button" onclick="mul()">Multiply</button>
<button type="button" onclick="div()">Divide</button>
<p id="demo" style="color:red;font-size:20px"></p>

Upvotes: 1

Views: 74

Answers (6)

c137
c137

Reputation: 65

We dont need to write each function for each calculation, Try to reuse the code insted of re-writing it, Since you are trying to execute the same logic but only the arithmetic operators are changed in each function you defined, we can write a single function and change the operators depending on the calculations we intend to perform. Below code uses function calculate(operation) which takes operation as a parameter and gives us the result depending on the argument passed.

Always try to use let insted of var as let has block scope but var doesn't
Also for better code readability and consistency follow naming conventions to name your variables

function calculate(operation){
    let total;
    
    let value1 = document.getElementById('Value1').valueAsNumber;
    let value2 = document.getElementById('Value2').valueAsNumber;
    
    if(operation == 'add') total = value1 + value2;
    else if(operation == 'sub') total = value1 - value2;
    else if(operation == 'multiply') total = value1 * value2;
    else if(operation == 'devide') total = value1 / value2;

    document.getElementById('demo').innerHTML = "Total: " + total;
}
<body style="text-align:center"><br><br>
    First Number: <input type="number" id="Value1"><br><br>
    Second Number: <input type="number" id="Value2"><br><br>
    <button type="button" onclick="calculate('add')">Add</button>
    <button type="button" onclick="calculate('sub')">Subtract</button>
    <button type="button" onclick="calculate('multiply')">Multiply</button>
    <button type="button" onclick="calculate('devide')">Divide</button>
    <p id="demo" style="color:red;font-size:20px"></p>
</body>

Upvotes: 0

HoldOffHunger
HoldOffHunger

Reputation: 20881

Some suggestions:

  • Let's separate the calculating from the formatted displaying. We'll have one function to calculate (+, -, /, *) and a different function to handle innerHTML-setting.
  • Since the calculations are all in one function, that removes the redundancy of multiple document.getElementById calls throughout your code.
  • We can use a switch() statement to determine the operation to apply, although a standard if()/else() also would work.

Working demo...

function operate(e) {
    var Total = getOperationTotal(e);
    document.getElementById('demo').innerHTML = "Total: " + Total;
}

function getOperationTotal(e) {
    var Value1 = document.getElementById('Value1').valueAsNumber;
    var Value2 = document.getElementById('Value2').valueAsNumber;
    var total;
    switch(e.target.id) {
         case 'add-button':
               total = Value1 + Value2;
               break;
         case 'subtract-button':
               total = Value1 - Value2;
               break;
         case 'multiply-button':
               total = Value1 * Value2;
               break;
         case 'divide-button':
               total = Value1 / Value2;
               break;
    }
    return total;
}
First Number: <input type="number" id="Value1"><br><br>
Second Number: <input type="number" id="Value2"><br><br>

<button id="add-button" type="button" onclick="operate(event)">Add</button>
<button id="subtract-button" type="button" onclick="operate(event)">Subtract</button>
<button id="multiply-button" type="button" onclick="operate(event)">Multiply</button>
<button id="divide-button" type="button" onclick="operate(event)">Divide</button>
<p id="demo" style="color:red;font-size:20px"></p>

Upvotes: 1

Tatul Mkrtchyan
Tatul Mkrtchyan

Reputation: 371

    <body style="text-align:center"><br><br>
        First Number: <input type="number" id="Value1"><br><br>
        Second Number: <input type="number" id="Value2"><br><br>
        <button type="button" onclick="action(this)">Add</button>
        <button type="button" onclick="action(this)">Subtract</button>
        <button type="button" onclick="action(this)">Multiply</button>
        <button type="button" onclick="action(this)">Divide</button>
        <p id="demo" style="color:red;font-size:20px"></p>
        <script>
            action = (e) => {
                const Value1 = document.getElementById('Value1').valueAsNumber;
                const Value2 = document.getElementById('Value2').valueAsNumber;
                let Total = null;
                switch(e.textContent) {
                    case 'Add':
                       Total = Value1 + Value2;
                       break;
                    case 'Subtract':
                       Total = Value1 - Value2;
                       break;
                    case 'Multiply':
                       Total = Value1 * Value2;
                       break;
                    case 'Divide':
                       Total = Value1 / Value2;
                       break;
                }
                
                document.getElementById('demo').innerHTML = `Total: ${Total}`;
            }
        </script>
    </body>

Upvotes: 0

T.J. Crowder
T.J. Crowder

Reputation: 1074258

Is there a way I could write these before I make each individual function?

You can get the elements just once:

const input1 = document.getElementById("Value1");

Then when you need its value:

const value1 = input1.valueAsNumber;

In general, you can usually write a function to avoid duplicated logic. Probably not appropriate in this case, but for example:

function getValueAsNumber(id) {
    return document.getElementById(id).valueAsNumber;
}

then

const value1 = getValueAsNumber("Value1");

A couple of side notes:

  • In JavaScript and related languages, the overwhelming convention is to use an initial lower case letter for variables that don't refer to constructor functions. So value1 rather than Value1, value1Element rather than Value1Element, etc.
  • var is no longer best practice in JavaScript. In new code, prefer let or const because of their more useful scoping.
  • When putting just plain text in an element, you're better off using textContent rather than innerHTML, because the browser doesn't try to parse the text you give it as HTML.
  • onxyz-attribute-style event handlers are not best practice, not least because the functions they call have to be globals, and the global namespace is crowded. Consider using modern event handling (addEventListener and the like).

Just for an example, here's your code with some of the above applied, but without going overboard:

const gid = id => document.getElementById(id);
const input1 = gid("Value1");
const input2 = gid("Value2");
const demo = gid("demo");

gid("add").addEventListener("click", () => {
    const total = input1.valueAsNumber + input2.valueAsNumber;
    demo.textContent = "Total: " + total;
});

gid("sub").addEventListener("click", () => {
    const total = input1.valueAsNumber - input2.valueAsNumber;
    demo.textContent = "Total: " + total;
});

gid("mul").addEventListener("click", () => {
    const total = input1.valueAsNumber * input2.valueAsNumber;
    demo.textContent = "Total: " + total;
});

gid("div").addEventListener("click", () => {
    const total = input1.valueAsNumber / input2.valueAsNumber;
    demo.textContent = "Total: " + total;
});
First Number: <input type="number" id="Value1"><br><br>
Second Number: <input type="number" id="Value2"><br><br>
<button type="button" id="add">Add</button>
<button type="button" id="sub">Subtract</button>
<button type="button" id="mul">Multiply</button>
<button type="button" id="div">Divide</button>
<p id="demo" style="color:red;font-size:20px"></p>

Or we could abstract away everything but the operation itself (just for fun):

const gid = id => document.getElementById(id);
const input1 = gid("Value1");
const input2 = gid("Value2");
const demo = gid("demo");
const clickHandler = (id, operation) => {
//                        ^^^^^^^^^
    gid(id).addEventListener("click", () => {
        const total = operation(input1.valueAsNumber, input2.valueAsNumber);
        //            ^^^^^^^^^
        demo.textContent = "Total: " + total;
    });
};
clickHandler("add", (a, b) => a + b);
//                  ^^^^^^^^^^^^^^^
clickHandler("sub", (a, b) => a - b);
clickHandler("mul", (a, b) => a * b);
clickHandler("div", (a, b) => a / b);
First Number: <input type="number" id="Value1"><br><br>
Second Number: <input type="number" id="Value2"><br><br>
<button type="button" id="add">Add</button>
<button type="button" id="sub">Subtract</button>
<button type="button" id="mul">Multiply</button>
<button type="button" id="div">Divide</button>
<p id="demo" style="color:red;font-size:20px"></p>

You could even go further than that, but it starts getting hard to follow. :-)

Upvotes: 2

T. Jami
T. Jami

Reputation: 1016

You can define global variables and additionally add onchange functions for the input, so as soon as you change the number the value changes in JS too.

    var Value1 = document.getElementById('Value1').valueAsNumber;
    var Value2 = document.getElementById('Value2').valueAsNumber;
    function updateNr(id){
        (id === 1) ? Value1 = document.getElementById('Value1').valueAsNumber : Value2 = document.getElementById('Value2').valueAsNumber;
    }
    function add() {
        var Total = Value1 + Value2;
        postAnswer(Total);
    }
    function sub() {
        var Total = Value1 - Value2;
        postAnswer(Total);
    }
    function mul() {
        var Total = Value1 * Value2;
        postAnswer(Total);
    }
    function div() {
        var Total = Value1 / Value2;
        postAnswer(Total);
    }
    function postAnswer(total){
        document.getElementById('demo').innerHTML = "Total: " + total;
    }
First Number: <input onchange="updateNr(1)" type="number" id="Value1"><br><br>
Second Number: <input onchange="updateNr(2)" type="number" id="Value2"><br><br>
<button type="button" onclick="add()">Add</button>
<button type="button" onclick="sub()">Subtract</button>
<button type="button" onclick="mul()">Multiply</button>
<button type="button" onclick="div()">Divide</button>
<p id="demo" style="color:red;font-size:20px"></p>

Upvotes: 0

dw_
dw_

Reputation: 1827

You can make a function to get the value of an element by ID, and re-use it, also you can create 1 function that will show the total as well.

function showTotal(total) {
  document.getElementById('demo').innerHTML = "Total: " + total;
}

function getValue(id) {
  return document.getElementById(id).valueAsNumber;
}

function add() {
    showTotal(getValue("Value1") + getValue("Value2"));
}
function sub() {
    showTotal(getValue("Value1") - getValue("Value2"));
}
function mul() {
    showTotal(getValue("Value1") * getValue("Value2"));
}
function div() {
    showTotal(getValue("Value1") / getValue("Value2"));
}
<body style="text-align:center"><br><br>
    First Number: <input type="number" id="Value1"><br><br>
    Second Number: <input type="number" id="Value2"><br><br>
    <button type="button" onclick="add()">Add</button>
    <button type="button" onclick="sub()">Subtract</button>
    <button type="button" onclick="mul()">Multiply</button>
    <button type="button" onclick="div()">Divide</button>
    <p id="demo" style="color:red;font-size:20px"></p>
</body>

Upvotes: 1

Related Questions