CODEWITHSUNDEEP
javascriptjqueryradio-button
potts
potts

Reputation: 155

Combining a Javascript function

I'm trying to create a voting style system where the user selects 2 numbers from 2 separate groups of radio buttons.

I've been able to do this, however I don't feel it's as optimised as it should be:

http://jsfiddle.net/f54wpLzg/11/

function updateQuality() {
var quality = document.getElementsByClassName('quality');
for (var i = 0, length = quality.length; i < length; i++) {
    if (quality[i].checked) {
        totalQuality = parseInt(quality[i].value);
        break;
    }
}
qualityVal = totalQuality;    
document.getElementById('totalQuality').innerHTML = qualityVal;
}

Is there anyway to combine the functions? I'd prefer not to have the

onclick="updateService();

On every single radio button as well...

Upvotes: 0

Views: 52

Answers (3)

chukingko
chukingko

Reputation: 1

you could just add a parameter inside the function then put a condition inside something like this..

function updateVote(str) {
    var data = document.getElementsByClassName(str);
    for (var i = 0, length = data.length; i < length; i++) {
        if (data[i].checked) {
           totalCount = parseInt(data[i].value);
           break;
        }
    }
    val = totalCount; 
    if(str=="quality")
       document.getElementById('totalQuality').innerHTML = val;
    }
    else{
       document.getElementById('totalService').innerHTML = val;
    }

on your html file..

onclick="updateVote('service')"

or

onclick="updateVote('quality')"

Upvotes: 0

Gabriele Petrioli
Gabriele Petrioli

Reputation: 196002

Since you are using jQuery you can remove the onclick attributes and do

$('#quality').on('change', '.quality', updateQuality);
$('#service').on('change', '.service', updateService);

in your script

To use a single method you could alter a bit your html to specify a target for each group (to display the value)

<div id="quality" data-target="#totalQuality">
    <input type="radio" class="quality" name="quality" value="1">
    <input type="radio" class="quality" name="quality" value="2" >
    <input type="radio" class="quality" name="quality" value="3">
</div>

<div id="service" data-target="#totalService">
    <input type="radio" class="service" name="service" value="1">
    <input type="radio" class="service" name="service" value="2">
    <input type="radio" class="service" name="service" value="3">
</div>

And then you can just do 

function update() {
    var target = $(this).closest('[data-target]').data('target');
    $(target).text(this.value);
}

$('#quality, #service').on('change', 'input', update);

But it will not update global variables (if you required those)

Upvotes: 1

Rory McCrossan
Rory McCrossan

Reputation: 337570

You can both simplify and DRY up your code. Firstly add a data attribute to the containers to identify which element should be used to display the total:

<div id="quality" data-target="totalQuality">
    <input type="radio" class="quality" name="quality" value="1" />
    <input type="radio" class="quality" name="quality" value="2" />
    <input type="radio" class="quality" name="quality" value="3" />
</div>
<div id="service" data-target="totalService">
    <input type="radio" class="service" name="service" value="1" />
    <input type="radio" class="service" name="service" value="2" />
    <input type="radio" class="service" name="service" value="3" />
</div>

<br>

<span id="totalQuality">0</span>
<span id="totalService">0</span>

Then you can remove the onclick attribute and use jQuery to attach a single event handler to all the radios:

$('#quality input, #service input').change(function() {
    var total = $(this).parent().data('target');
    $('#' + total).html($(this).val());
});

Example fiddle

Upvotes: 2

Related Questions