Undefined - Trying to pass a value from a form to a JS function using jQuery and getting it very wrong

Question

I'm trying to learn how to use JS in order to create a unit converter for a site I'm working on.
I did have intentions of trying to accomplish it using PHP but someone pointed out how inefficient it would be and so I'm now trying to learn JS to carry out the same tasks.

I've written a very small test function to add two numbers, it worked fine. I then adjusted it slightly to take in a few more params and to check a couple of conditions, again that worked fine - I created a new object and passed the variables in directly.

I now need to pass the values from the form that I have into this function in order to compute the sum and output the result. I keep getting an error of 'undefined'. I've googled and read but can't seem to find a solution.

so far I have:

<script type="text/javascript">

    function Convert(from, to, units){

        this.from = $("#from").val();
        this.to = $("#to").val();
        this.units = $("#units").val();
    }

    Convert.prototype.convertThem = function(){

        if(this.from == "degC"){
            if(this.to == "degF"){
                return this.units * 347956757524;
            }
        }
    }

    calcTempTest = new Convert(this.from, this.to, this.units);
    alert(calcTempTest.convertThem());
    console.log(calcTempTest);

</script>

Could anyone tell me what I'm doing wrong please? The 'to','from' and 'units' are the id's from the form.

The Form:

<div class="form">
        <label for="units">Units:</label>
        <input type="text" name="units" id="units" class="required digits"  />
    </div>

    <div class="form">
        <label for="from">Convert From:</label>
            <select name="from" id="from">
                <option value="from">-Select an Option-</option>
            </select>
    </div>

    <div class="form">
        <label for="to">Convert Into:</label>
        <select name="to" id="to">
            <option value="to">-Select an Option-</option>
        </select>
    </div>

    <div class="form">
        <label> &nbsp; </label>
        <input type="submit" name="submit" value="Convert!" />
    </div>

many thanks.

What is the exact error you're getting? If you press F12 in Chrome, firefox or Opera you should see the console with the exact error. If it's $ is undefined than you're probably not including the jQuery script correctly. — HMR, Jul 04 '13 at 13:48
i have : included. The error doesn't appear in the console. The alert box appears on page load with undefined and once again when I submit the form. — null, Jul 04 '13 at 13:50
Also, and most importantly I guess : is this anywhere near the 'correct' way of accomplishing this task? — null, Jul 04 '13 at 13:51
@SteveGreen I've updated my answer, you're getting there. One thing though; if that script is in the page header it will be executed before any of your html input elements are available. Better not call it directly but call it when a button is clicked. — HMR, Jul 04 '13 at 13:55
@SteveGreen A couple of points to begin with: The constructor is not using its parameters, you should change that. You probably should store a reference to the nodes and not their values in the `Convert` instance and only inquire the value when calculation is due (attach an event listener to a button or something). Your `convertThem` method does not always return anything, so this is probably the `undefined` that you're seeing. — MasterAM, Jul 04 '13 at 13:59
I've updated my answer with some code that "does what I think you want". When you have more methods you might want to check out how to optimize it. — HMR, Jul 04 '13 at 14:08

score 1 · Accepted Answer · edited Jun 20 '20 at 09:12

Explanation

Your select selected option value onLoad both are "from" and "to". Since these are not equal to "degF" and "degC", your assignments won't go on, the resulting variable will be undefined since no value will be asssigned to it.

Solution

Add several option to your select or change their default value. I also added a default value to the input.

HTML

<input type="text" name="units" id="units" value="12" class="required digits"  />

<option value="degC">-Select an Option-</option>

<option value="degF">-Select an Option-</option>

EDIT

I have added a JSFiddle here which executes the script on the button click with the following modifications to JavaScript:

NOTE: I also added the real formula.

JavaScript/jQuery

 $('input[name="submit"]').click(function () {
    var c = new Convert();
    alert(c.convertThem());
});

function Convert() {

    this.from = $("#from").val();
    this.to = $("#to").val();
    this.units = $("#units").val();
}

Convert.prototype.convertThem = function () {

    if (this.from == "degC") {
        if (this.to == "degF") {
           return this.units * 1.8 + 32;
        }
    }
}

thank you! Brilliant explanation and thanks for taking the time with the fiddle. I haven't set values to the options as they're getting auto populated by a JSON but it works brilliantly. Thanks! — null, Jul 04 '13 at 14:07

score 0 · Answer 2 · edited May 23 '17 at 10:25

I think when you create the convert object you're trying to pass variables that don't exist:

calcTempTest = new Convert(this.from, this.to, this.units);

I'm pretty sure this stands for window at that point and windw.from is undefined. You don't seem to be doing anything with these values anyway so you could change it to:

calcTempTest = new Convert();

Maybe the following answer could help you out with what this stands for in JS: Prototypical inheritance - writing up

Here is some minimally working code:

<!DOCTYPE html>
<html>
<head>
    <title>test</title>
    <script type="text/javascript" src="jquery-1.10.1.js"></script>
</head>
<body>


<div class="form">
        <label for="units">Units:</label>
        <input type="text" name="units" id="units" class="required digits"  />
    </div>

    <div class="form">
        <label for="from">Convert From:</label>
            <select name="from" id="from">
                <option value="degC">degC</option>
            </select>
    </div>

    <div class="form">
        <label for="to">Convert Into:</label>
        <select name="to" id="to">
            <option value="degF">degG</option>
        </select>
    </div>

    <div class="form">
        <label for="output">Output:</label>
        <input type="text" id="output" />
    </div>

    <div class="form">
        <label> &nbsp; </label>
        <input type="submit" id="subm" name="submit" value="Convert!" />
    </div>
<script type="text/javascript">
    (function () {
        function Convert(from, to, units) {
            // when convert is created set a reference to the input elements
            this.$from = $("#from");
            this.$to = $("#to");
            this.$units = $("#units");
            this.$output = $("#output");
        }

        Convert.prototype.convertThem = function () {
            // this.$... is a jQuery object containing the input elements
            if (this.$from.val() == "degC") {
                if (this.$to.val() == "degF") {
                    this.$output.val( this.$units.val() * 347956757524);
                }
            }
        }

        calcTempTest = new Convert();
        $("#subm").on("click", null, null, function () {
            calcTempTest.convertThem();
        });
    })();//anonymous funciton, no variables in global scope
</script>
</body>
</html>

I, um, also may have had the script in the head too...maybe. — null, Jul 04 '13 at 14:06
@SteveGreen Looks like a good attempt, if you're just starting out then it'll take some time to understand how stuff works. You should try to see the answer on prototype (link above). It might make your head explode but once you understand it it'll be very valuable. You can copy and paste code from that link in the commandline of the console (press F12 in the browser) and run it (press enter in Chrome or run in Firefox with firebug) It's a fast way to fiddle around with code. — HMR, Jul 04 '13 at 14:14
thanks! I'm now checking it out. Thanks also for the code example, I'm combining them both into what I hope is the 'best' way. — null, Jul 04 '13 at 14:29
@SteveGreen You're welcome. If you have your code working and would like someone have a look at it to see if can be done "better" you can post it on code review: http://codereview.stackexchange.com/ — HMR, Jul 04 '13 at 14:32
I'll be making use of that quite a bit I expect. I'm unable to get the code above working. I tried changing a few things but nothing seems to be happening. — null, Jul 04 '13 at 14:46
@SteveGreen note that it uses jQuery, the first javascript tag includes the jQuery library so you have to point that to the jQuery.js file. Other than that you don't need to change anything. I suggest trying to use netbeans as you can right click a file and check it's history (keeps a copy on every save) that way you can compare with history and check what is different whenever something unexpected breaks. — HMR, Jul 04 '13 at 15:33
I really enjoy using netbeans. I've used it a ton in the past but when starting this project phpStorm was recommended. It's a pretty decent IDE. I'm using a sandbox site for all testing and keep regular backups. Time machine has saved me several times, that and rsi injury from ctrl+z combo. I did notice the jQuery, I like how they work together. The simple .val() was an example that is quite nice in that respect. current problem. The option boxes are populated via jquery and json. When I click convert there is a refresh and the answer doesn't display - debugging in a foreign language -.- — null, Jul 04 '13 at 16:12

score 0 · Answer 3 · answered Jul 04 '13 at 14:51

There are several issues with your code. Most of them have been resolved in the accepted answer, but I wanted to provide some more insights that would help you create more reusable code in the future.

Since I have already created a jsfiddle with my own example, it will be a shame to let it go to waste so I will post it anyway with some comments.

Using constructor parameters

function Convert(from, to, units, res){
    this.from = from;
    //etc...
}

Passing parameters to an object's constructor (and using them) makes it more reusable. You did not use the passed parameters and the selected answer used what I assume was your original solution (hard-coding the element values into the object upon construction).

This way you can have multiple instances of the converter on the same page, you can put its code in an external file as it gets more complex and only put the instantiation logic in the page itself (if your page structure changes, there is no need to change the external file, just update the provided constructor parameters).

Storing node references instead of values

The other thing I wanted to point out is the way the calculation is done.

Your implementation requires a new object to be created for each calculation. I find it much better to create a single Converter and obtain the values only when required. That it the reason I stored a reference to the form field DOM nodes and did not store their values.

$("#btnConvert").click(calcTempTest.convertThem.bind(calcTempTest));

I used bind(...) in the click attachment to preserve the object's scope.

Good luck!

thank you for the sample and the advice, all that you mentioned makes perfect sense and I'd be a fool not to utilise the method you suggested. thanks again! — null, Jul 04 '13 at 15:32
You're welcome. I would recommend getting some in-depth knowledge about JS and learn JQuery's way of extending objects and creating plugins. — MasterAM, Jul 08 '13 at 13:22