HTML radio buttons

From: af (CAER)11 Feb 2011 14:30
To: Radio 68 of 95
quote:
And to be really boring and on topic - attached is an anonymised version of what I was trying to do, although it would then be extended/copy+pasted for multiple countries.

I like how it looks, but I'm sure that the coding is terribly inefficient and when actually extended for lots of countries would probably become unmanageable.

If something can be copied+pasted to deal with multiple things, it's almost always better to write it once to handle multiple things instead, because as you said, it does indeed become unmanageable.

This sort of thing, repeated 20 (!) times:
code:
//Check which checkboxes/options are selected
var f5 = (document.listofoptions.elements['Check5'].checked);
//Check if 'f5' is "true" and set to price, else set to zero
if(f5 == true)
{
var f5 = 50;
var w5 = 30;
}
else
{
var f5 = 0;
var w5 = 0;
}
Could be replaced with a loop like this:
javascript code:
// Checkbox values must be in the format: "normal,wholesale"
var checkBoxes = document.querySelectorAll("input[type=checkbox]"),
    max = checkBoxes.length, values,
    i, total = 0, totalWholesale = 0;
 
for (i = 0; i < max; i += 1) {
    if (checkBoxes[i].checked) {
        values = checkBoxes[i].value.split(",");
 
        // The + on the end is to convert the value to a number.
        total += +values[0];
        totalWholesale += +values[1];
    }
}


If you were using IE < 8, btw, querySelectorAll() would need to be replaced with a custom function to enumerate the checkboxes, which shouldn't be too complicated; probably something like this:
javascript code:
var getCheckboxes = function () {
    var i, elements = document.getElementsByTagName("input"),
        max = elements.length,
        result = [];
 
    for (i = 0; i < max; i += 1) {
        if (elements[i].type === "checkbox") {
            result.push(elements[i]);
        }
    }
    return result;
};
EDITED: 11 Feb 2011 17:33 by CAER
From: 99% of gargoyles look like (MR_BASTARD)11 Feb 2011 15:14
To: Matt 69 of 95
That's App£€, and no, I didn't know that.
From: 99% of gargoyles look like (MR_BASTARD)11 Feb 2011 15:17
To: af (CAER) 70 of 95
i += 1? i++?
From: 99% of gargoyles look like (MR_BASTARD)11 Feb 2011 15:18
To: milko 71 of 95
I wouldn't bother if I were you, it's trivially easy to circumvent.
EDITED: 11 Feb 2011 15:18 by MR_BASTARD
From: af (CAER)11 Feb 2011 15:25
To: 99% of gargoyles look like (MR_BASTARD) 72 of 95
I prefer i += 1 as I find it more readable, and easier to adjust.
From: milko11 Feb 2011 15:28
To: 99% of gargoyles look like (MR_BASTARD) 73 of 95
well, it's quite easy to add most of the workarounds too. And why would anyone bother? I don't really know why anyone would type it in the first place mind you, so I maybe at an understanding-disadvantage here.
From: 99% of gargoyles look like (MR_BASTARD)11 Feb 2011 16:10
To: af (CAER) 74 of 95
Oddly enough, I find i++ easier to read, although I agree it's less flexible.
From: 99% of gargoyles look like (MR_BASTARD)11 Feb 2011 16:11
To: milko 75 of 95
quote: milko
And why would anyone bother?

To maintain freedom of speech, deny MOD MADNESS, and retain the original meaning. Except I'm not sure what it is.
From: Radio11 Feb 2011 16:18
To: af (CAER) 76 of 95
So would that pull the values from the rows of the table, or would I have to rewrite the table to support the function?
From: af (CAER)11 Feb 2011 17:27
To: Radio 77 of 95
Well as it is you'd have to rewrite the table, but this would take the values from the table itself:
javascript code:
var checkBoxes = document.querySelectorAll("input[type=checkbox]"),
    max = checkBoxes.length,
    i, total = 0, totalWholesale = 0;
 
for (i = 0; i < max; i += 1) {
    if (checkBoxes[i].checked) {
        // Get values directly from table cells.
        total += +checkBoxes[i].parentElement.
            previousElementSibling.innerHTML;
        totalWholesale += +checkBoxes[i].parentElement.
            previousElementSibling.previousElementSibling.innerHTML;
    }
}


edit: just realised the + should be at the start of the variable names, not the end. The + at the start is a forced type conversion; an alternative is to use:
javascript code:
total += parseInt(checkBoxes[i].parentElement.previousElementSibling.innerHTML, 10);


Edit again:
Another thing I thought of was that using jQuery needn't be any hassle - you can just include this line on your page somewhere before your current JS code:
HTML code:
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.5.0/jquery.min.js"></script>


edit3: oo, jQuery 1.5 is out :O
EDITED: 11 Feb 2011 17:41 by CAER
From: Matt11 Feb 2011 19:12
To: af (CAER) 78 of 95
quote:
edit3: oo, jQuery 1.5 is out


Hooray. I wonder if it breaks as much stuff as 1.3 to 1.4 did.
From: Radio14 Feb 2011 12:35
To: af (CAER) 79 of 95

So what you're saying is that I should remove all the 20 individual inspections of the checkboxes, and replace with just this code? And that it will produce the same page?

 

That doesn't work, and I'm not sure what else I'd need to do.

From: 99% of gargoyles look like (MR_BASTARD)14 Feb 2011 13:51
To: af (CAER) 80 of 95

It's probably just me being anally retentive, but I bloody hate innerHTML, it's not part of the DOM and it's queer.

 

And by "queer" I mean gay and homosexual, not as in "by jove, that's a rum do and no mistake".

 

And by "gay" I mean queer and homosexual, not "having a gay old time, dahling".

 

It's a wonderful thing, this English language malarkey, isn't it?

From: Peter (BOUGHTONP)14 Feb 2011 13:52
To: Radio 81 of 95
If he'd used jQuery it would have worked! ;)

What does the error in the console say?
From: Radio14 Feb 2011 13:58
To: Peter (BOUGHTONP) 82 of 95

Object doesn't support this property or method (line 104, char 1)

 

Line 104 is:
var checkBoxes = document.querySelectorAll("input[type=checkbox]"),

From: Peter (BOUGHTONP)14 Feb 2011 14:09
To: Radio 83 of 95
Are you using an old version of IE?
From: af (CAER)14 Feb 2011 14:53
To: 99% of gargoyles look like (MR_BASTARD) 84 of 95
What would you suggest instead?
From: af (CAER)14 Feb 2011 15:04
To: Radio 85 of 95
Add this code to the top of the <script> section:
javascript code:
var getCheckboxes = function () {
    var i, elements = document.getElementsByTagName("input"),
        max = elements.length,
        result = [];
 
    for (i = 0; i < max; i += 1) {
        if (elements[i].type === "checkbox") {
            result.push(elements[i]);
        }
    }
    return result;
};
Then change the line with the error so it reads:
javascript code:
var checkBoxes = getCheckboxes,
If that still doesn't work, post the code you have and I'll see if I can spot the problem.

Btw, the reason it's best to put all your var declarations at the start of the function is that it helps you get clear in your head what the function needs and does. When you start declaring them inline whenever and wherever, it becomes difficult to remember what does what.
From: 99% of gargoyles look like (MR_BASTARD)14 Feb 2011 15:38
To: af (CAER) 86 of 95
I've kind of lost the plot as to why a table value is being extracted, but if it's associated with a check box I'd be tempted to read the name, value, and checked attributes of the input.
EDITED: 14 Feb 2011 15:39 by MR_BASTARD
From: Radio14 Feb 2011 16:17
To: af (CAER) 87 of 95

Here you go - it now complains that f5 isn't defined (which sort of makes sense as I used to use that to calculate my totals).
I could remove the totaliser line (line 135) but no idea what to put in it's place.

 

As for why I'm reading a value from a table, have a look at the file in IE and it should make more sense. This is about choosing a base product from a choice of 4, and then adding various options.
Radio boxes are used for the initial selection, and then checkboxes for the rest. The total then displays the price for your selection.

Attachments: