Click-through rate calculator using jQuery

Question

I created a calculator for CTR with jQuery. The calculator is working just the way I want, but I'm quite new to jQuery and I wonder if I am using jQuery effectively.

The goal of the code is that if 2 values are entered and the button is clicked then the correct function is called and the output's background colour will change.

<div class="container">
<div class="alert alert-info" role="alert">
    <span class="align-middle">CTR Calculator</span>
</div>


 <div>         
    <label for="v1">Number of Impressions:</label> <br/> 
    <input id="v1" type="text" style="width:10em;"/> <br/>

    <label for="v2">Number of Clicks:</label> <br/>
    <input id="v2" type="text" style="width:10em;"/> <br/>

    <label for="v3">CTR:</label><br/> 
    <input id="v3" type="text" style="width:10em;"/> <br/>

<div>
    <button id ="btn" type="button" class="btn btn-secondary">Calculate</button>
</div>
<div class="alert alert-info result" role="alert">
        <span id="result" class="align-middle"></span>
</div>  

  $(document).ready(function(){
      $("#btn").on("click",function(event){
        if ($("#v1").val() === '' && $("#v2").val() === '' && $("#v3").val() === '') {
            $('#v1,#v2,#v3').css('background-color', 'transparent');
            alert("Please enter atleast 2 values!");  
        } else if ($("#v1,#v2").val() === '' || $("#v1,#v3").val() === '' || $("#v2,#v3").val() === '') {
            $('#v1,#v2,#v3').css('background-color', 'transparent');
            alert("Please enter atleast 2 values!");
        } else if ($("#v1").val() === '') {
            var mathImp = (parseInt($('#v2').val()) / parseInt($('#v3').val())) * 100;
            $('#v1').val(mathImp);
            $('#v1').css('background-color', 'beige');
            $('#v2,#v3').css('background-color', 'transparent');
        }  else if ($("#v2").val() === '') {
            var mathClicks = (parseInt($('#v1').val()) * parseInt($('#v3').val())) / 100;
            $('#v2').val(mathClicks);
            $('#v2').css('background-color', 'beige');
            $('#v1,#v3').css('background-color', 'transparent');
        } else if ($("#v3").val() === '') {
            var mathCTR = (parseInt($('#v2').val()) / parseInt($('#v1').val())) * 100;
            $('#v3').val(mathCTR);
            $('#v3').css('background-color', 'beige');
            $('#v1,#v2').css('background-color', 'transparent');
        } else {
            alert("Please enter only 2 values!");
        }
     });
});

Answer 1

Used native DOM API

You don't need jQuery.

• It isolates you from learning the very mature native DOM interface.
• It encourages bad code practices, both by its design, and the countless bad examples on the web.
• It bloats your source code with repetitive notation making it harder to read and more prone to bugs.
• It adds to the resource load of every page, consuming more power, reducing client battery life, warming the world, and forcing cats and dogs to sleep together.

As you are a beginner your time is much better spent learning to use the native DOM directly. This will give you wider, future proof skill set.

Direct element reference

If you use element ids correctly (they must be unique) you can use the id to directly reference elements in javascript. This is supported by all browsers.

<input id="v1" type="text" style="width:10em;"/>
<script>
    v1.value = "Hi all.";

    // same as
    $("#v1").val("Hi all.");
</script>

ES6+

Your code can be rewritten using ES6+

addEventListeren("load", () => {

    const color = "beige";
    const setBG = (col, ...els) => { 
        for (const el of els) { el.style.background = col } 
    };
    const sumVals = (...els) => els.reduce((sum, el) => sum + el.value !== "" ? 1 : 0, 0);

    btn.addEventListener("click", () => {

        setBGCol("transparent", v1, v2, v3);
        if (sumVals(v1, v2, v3) === 2) {
            if (v1.value === '') {
                v1.value = v2.value / v3.value * 100;
                setBG(color, v1);
            } else if (v2.value === '') {
                v2.value = v1.value * v3.value / 100;
                setBG(color, v2);
            } else {
                v3.value = v2.value / v1.value * 100;
                setBG(color, v3);
            }
        } else {
            alert("Please enter only 2 values!");
        }
    });
});

ES5

Or for ES5 compatible with IE11

addEventListeren("load", function() {
    var color = "beige";
    function setBG(col, elements) {
        elements.foreach(function(el) { 
            el.style.backgroundColor = col 
        });
    };
    function hasValue(element) { 
        return element.value !== "" ? 1 : 0; 
    }
    btn.addEventListener("click", function() {
        setBG("transparent", [v1, v2, v3]);
        if (hasValue(v1) + hasValue(v2) + hasValue(v3) === 2) {
            if (v1.value === '') {
                v1.value = v2.value / v3.value * 100;
                setBG(color, [v1]);
            } else if (v2.value === '') {
                v2.value = v1.value * v3.value / 100;
                setBG(color, [v2]);
            } else {
                v3.value = v2.value / v1.value * 100;
                setBG(color, [v3]);
            }
        } else {
            alert("Please enter only 2 values!");
        }
    });
});

Answer 2

I think this style is what causes a lot of the "jquery mess" you hear people talk about. Let me try to explain why.

What's going on is that the code tries to handle every possible view state change. For example, when you want to highlight a certain input, you also need to consider that one or both of the other inputs are highlighted already and so you need to hide them. What you can do instead is to "reset" the view state before every run and in that way only have to consider the one case. This is called declarative programming, since you basically declare what your view will look like given a certain state, but you don't care about moving from one state to another.

In your case, you would "reset" your view state like so:

$('#v1,#v2,#v3').css('background-color', 'transparent')

Adding this simple line at the start of your click-handler removes a ton of noise from the rest of the code.

Here's a couple of other things that can be improved:

1. Only use ', not " as well
2. Don't duplicate jquery selectors
3. Use better names than v1,v2,v3
4. If you only care that the user inputs two values, check that (as opposed to checking exact combinations)

Here's one attempt at a refactoring (not tested):

$(document).ready(() => {
    $('#btn').on('click', () => {
        let $impressions = $('#v1')
        let $clicks = $('#v2')
        let $ctr = $('#v3')
        let inputs = [$impressions, $clicks, $ctr]

        inputs.forEach($input => $input.css('background-color', 'transparent'))

        let nFilled = inputs.filter($input => $input.val() !== '').length
        if(nFilled !== 2){
            alert('Please enter exactly 2 values!')
            return
        }

        let setCalculatedInput = ($input, value) => {
            $input.val(value)
            $input.css('background-color', 'beige')
        }

        if($impressions.val() === ''){
            setCalculatedInput($impressions, (parseInt($clicks.val()) / parseInt($ctr.val())) * 100)
        }else if($clicks.val() === ''){
            setCalculatedInput($clicks, (parseInt($impressions.val()) * parseInt($ctr.val())) / 100)
        }else if($ctr.val() === ''){
            setCalculatedInput($ctr, (parseInt($clicks.val()) / parseInt($impressions.val())) * 100)
        }
    })
})