3
\$\begingroup\$

I'm trying to write a little page which will help me - when it ever gets finished - in my job to get some stuff done easier and faster. I'd like some hints and tricks for better structuring the code, i.e. to better divide logic from the UI.

I use jQuery a bit (need to expand my knowledge of using it) and will finally probably use twitter bootstrap as a framework.

/*
* This section is about the "select" input elements
* 
* This should realize a cascading dropdown, which
* shows options in the second select input elemen
* "protocols" depending on the selection of a scanner
*
* Found this solution on the web an adopted it ab bit
* to my needs.
*/

function appendOptionToSelect(sel, opt) {
    try {
        sel.add(opt, null);
    } catch (e) {
        //for IE7 and earlier
        if (e.name == "TypeError") {
            sel.options[sel.options.length] = opt;
        } else {
            throw e;
        }
    }
};

function removeAllChildNodes(element) {
    if (element.hasChildNodes()) {
        while (element.childNodes.length >= 1) {
            element.removeChild(element.firstChild);
        };
    };
};

function selChanged(sel, data, dependentSel) {
    var selection = sel.options[sel.selectedIndex].value;
    var arrOptions = data[selection];
    var opt;
    removeAllChildNodes(dependentSel);
    for (var i in arrOptions) {
        opt = new Option(arrOptions[i]);
        appendOptionToSelect(dependentSel, opt);
}
};






// This should be the object which holds the protocols specific
// for a scanner
var scanner = {
    "scanner1" : ["--choose--", "protocol 1", "protocol 2", "protocol 3"],
    "scanner2" : ["--choose--", "protocol 2", "protocol 4", "protocol 5"],
    "scanner3" : ["--choose--", "protocol 1", "protocol 2", "protocol 5", "protocol 6"],
"scanner4" : ["--choose--", "protocol 3a", "protocol 4b", "protocol xyz"]
};

// Needed down in the eventhandlers
var selectScanner = document.getElementById("scanner");
var selectProtocol = document.getElementById("protocols");



// Calculation the age in years (found this on the web)
function getAge() {
    var dateOfExam = new Date(document.getElementById('dateOfExam').value);
    var dateOfBirth = new Date(document.getElementById('dateOfBirth').value);

function isLeap(year) {
    return year % 4 == 0 && (year % 100 != 0 || year % 400 == 0);
}

// age in days
var days = Math.floor((dateOfExam.getTime() - dateOfBirth.getTime()) / 1000 / 60 / 60 / 24);
var age = 0;
// calculating years
for (var y = dateOfBirth.getFullYear(); y <= dateOfExam.getFullYear(); y++) {
    var daysInYear = isLeap(y) ? 366 : 365;
    if (days >= daysInYear) {
        days -= daysInYear;
        age++;
        // just increment the year if there are enough days for a year
    }
}
return age;
}






/* Bodysurface and BMI
* 
* Ok, now it gets chaotic, this is my solution.
* The aim is, to calculate both BSA and BMI depending on the input-values from
* the elements "weight" and "height".
* I need to get
* - a string like (for BSA): 2,3 m2
* - a string like (for BMI): 25,5
* - global(?) variables which stores the floats for bmi and bsa
* I guess my way is weird...
*/

var bsa, bmi, kof;

function Kof(cm, kg) {
    var mosteller = Math.sqrt(Number(cm) * Number(kg) / 3600);
    var bmi = Number(kg) / Math.pow((Number(cm) / 100), 2);
    this.getStringMosteller = function() {
        return mosteller.toFixed(1).replace(/\./, ",") + " m<sup>2</sup>";
    };
    this.getStringBmi = function() {
        return bmi.toFixed(1).replace(/\./, ",");
    };
    this.getMosteller = function() {
        return mosteller;
    };
    this.getBmi = function() {
        return bmi;
    };
}


function setBmiAndBsa() {
    var cm = parseFloat($('#height').val());
    var kg = parseFloat($('#weight').val());
    if (!isNaN(cm) && !isNaN(kg)) {     // checking that there are numbers
        kof = new Kof(cm, kg);      
        bsa = kof.getMosteller();       // getting the floats for bsa
        bmi = kof.getBmi();             // and bmi

    // and writing the string values to the html-page
    $('#bsa').html(kof.getStringMosteller());
    $('#bmi').html(kof.getStringBmi());
};
}



$(document).ready(function() {

    // set the current date as the date of examination
    $('#dateOfExam').val(new Date().toJSON().substring(0, 10));

    // calculate the age when a date of birth is entered
    $('#dateOfBirth').change(function() {
        $('#age').html(getAge());       // and writing it to the page
    });


    // The eventhandlers for height and weight
    $('#height').change(function() {
        setBmiAndBsa();
    });
    $('#weight').change(function() {
        setBmiAndBsa();
    });
    /*
     * Why the heck aren't these handlers working?
     *
     * $('#selectScanner').change(function() {
     * selChanged(selectScanner, scanner, selectProtocol);
     * });
     * $('#selectProtocol').change(function() {
     * alert(selectProtocol.options[selectProtocol.selectedIndex].value);
     * });
     * 
     * I have to write them like this:
     */
});
selectScanner.onchange = function() {
    selChanged(selectScanner, scanner, selectProtocol);
};
selectProtocol.onchange = function() {
    alert(selectProtocol.options[selectProtocol.selectedIndex].value);
};
<!DOCTYPE html>
<html>
<head>
    <meta charset="utf-8">
    <title>Testpage</title>
</head>
<body>
    <div>
        <form>
            <legend>
                Step 1
            </legend>
            <label><h6>1. Choose Scanner:</h6></label>
            <select id="scanner">
                <option>choose</option>
                <optgroup label="Group 1">
                    <option value="scanner1">Test Scanner 1</option>
                    <option value="scanner2">Test Scanner 2</option>
                </optgroup>
                <optgroup label="Group 2">
                    <option value="scanner3">Test Scanner 3</option>
                    <option value="scanner4">Test Scanner 4</option>
                </optgroup>
            </select>
            <label><h6>2. Choose protocol:</h6></label>
            <select id="protocols">
                <option></option>
            </select>
        </form>
    </div>
    <div>
        <form>
            <legend>Step 2</legend>
            <label><h6>Date of Examination:</h6></label>
            <input type="date" id="dateOfExam" />
            <label><h6>Date Of Birth:</h6></label>
            <input type="date"id="dateOfBirth" />
            <span>Age: <span id="age"> </span> </span>
            <br>
            <label><input type="radio" name="optionsRadios" id="genderMale" value="male" checked />male</label>
            <label><input type="radio" name="optionsRadios" id="genderFemale" value="female" />female </label>
            <br />
            <h6>Height [cm] and weight [kg]:</h6>
            <input type="number" id="height" name"height" placeholder="cm" >
            <input type="number" id="weight" name="weight" placeholder="kg" >
            <br>
            <span>BSA (Mosteller): <span id="bsa"> </span> </span>
            <br>
            <span>BMI: <span id="bmi"> </span> </span>
        </form>
    </div>
    <script src="//ajax.googleapis.com/ajax/libs/jquery/1.8.2/jquery.min.js"></script>
    <script>
    window.jQuery || document.write('<script src="js/jquery-1.8.2.min.js"><\/script>')
    </script>
    <script src="js/main.js"></script>
</body>
</html>

\$\endgroup\$
6
  • \$\begingroup\$ If you're still looking for an answer, can you say whether your code works as intended or not? Are there any specific problems with it, other than the one about jQuery event handlers not working? \$\endgroup\$
    – Stuart
    Dec 21, 2012 at 23:54
  • 1
    \$\begingroup\$ Maybe you could also say what your motivation is for making the object Kof instead of just, say, having functions getBMI(cm, kg) and getBSA(cm, kg), and why you are storing bmi and bsa as global variables. \$\endgroup\$
    – Stuart
    Dec 22, 2012 at 0:09
  • \$\begingroup\$ Hello Stuart. Thank you for finding, viewing and answering to this question. Of course I'm still interested in any hints how to write better scripts. To answer your questions: The code works like intended, but it is just a snippet out of the project i'm trying to code. About my motivation for making the object Kof: I thought it is a possibility to structure the code a bit more, and it's a component i need on multiple pages. Actually i'm trying to develop a "patient" or "human" object with a lot of attributes and methods, but I tried to start simple. \$\endgroup\$
    – Phil
    Dec 23, 2012 at 16:20
  • \$\begingroup\$ I used bmi and bsa as global variables because i wanted to separate some logic (mainly generating reports). And that logic (partially in the eventhandlers - i dislike it that way and have to change it) needs to get these values. But while reading more about scripting i just reached the chapter about self invoking anonymous functions - surely I should start using them in some parts. By the way: I changed the structure of the site, so no more selecting scanners and protocols is necessary... Thanks again - Phil. \$\endgroup\$
    – Phil
    Dec 23, 2012 at 16:20
  • 1
    \$\begingroup\$ Okay. Then I have nothing to say about the code, it looks fine to me. I don't see anything 'weird' about it. If Kof becomes a person/patient object you might want to start recording the weight and height as properties of the object this.cm = cm; this.kg = kg. I would not bother having the functions getMosteller and getBMI until/unless you need them. You can write $('#height').change(setBMIandBSA) instead of $('#weight').change(function() { setBmiAndBsa();}) \$\endgroup\$
    – Stuart
    Dec 23, 2012 at 19:53

1 Answer 1

3
\$\begingroup\$

This is only about your HTML as I don't know much about JS. I guess your page will only be used internally by you, so it is not that important, but it may help others that come by.

  • legend must only be used in fieldset
  • label can only include "phrasing content", so h6 is not allowed there (in general I wouldn't use headings inside of the form at all)
  • you should "connect" the label and it's input/select: with the for attribute
  • omit <option>choose</option> as it is (probably) no valid choice
  • group the two gender radio boxes in a fieldset with a legend like "Select gender"
    • same with "Height [cm] and weight [kg]" (fieldset>legend instead of h6)
\$\endgroup\$
5
  • \$\begingroup\$ Tried to fix the points you mentioned above. Thank you again for your help! \$\endgroup\$
    – Phil
    Nov 21, 2012 at 15:31
  • \$\begingroup\$ Great. I don't think that my answer deserves the green checkmark, because I didn't review your JS. Feel free to give it to someone that might do that in the future. \$\endgroup\$
    – unor
    Nov 21, 2012 at 17:00
  • \$\begingroup\$ I'm a complete newbie regarding posting on stackexchange. I thought it might be a kind of saying: your answer helped me. But I'm going to take a look at the vote / check system of stackexchange. Thanks for the hint. \$\endgroup\$
    – Phil
    Nov 21, 2012 at 17:26
  • \$\begingroup\$ @Phil: It also means that you probably don't need any more help, because the checked answer solved all your questions. So it is often a good idea to wait some time for more answers until you accept an answer. \$\endgroup\$
    – unor
    Nov 22, 2012 at 11:05
  • \$\begingroup\$ in the meantime I read about the checking and voting features, but thanks again for clarifying that issue. Phil \$\endgroup\$
    – Phil
    Nov 22, 2012 at 14:31

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.