How can I elegantize this verbose jquery?

Question

I need to get the value of the checked checkbox (only one is allowed to be checked at a time), and I have this verbose code to do so:

if (!checkboxSelected) {
    return;
}

if($("#ckbx_produceusage").is(':checked')) {
    rptval = $('#ckbx_produceusage').val();
}
else if($("#ckbx_deliveryperformance").is(':checked')) {
    rptval = $('#ckbx_deliveryperformance').val();
}
else if($("#ckbx_fillrate").is(':checked')) {
    rptval = $('#ckbx_fillrate').val();
}
else if($("#ckbx_pricecompliance").is(':checked')) {
    rptval = $('#ckbx_pricecompliance').val();
}
setEmailAndGenerateValsForUnitReportPair(unitval, rptval);

Is there a way I can make this code less verbose without making it ungrokkable? I thought of dynamically adding a class to the checkbox that is checked, and then removing it from any that previously had that class added, and then getting the rptval based on which one is currently decorated with that class. That seems a bit smelly or Rubegoldbergesque, though, so am looking for a better solution.

UPDATE

For T.J. Crowder, here is the HTML (Razor/ASP.NET MVC style):

@foreach (var rpt in reports)
{
    @* convert id to lowercase and no spaces *@
    var morphedRptName = @rpt.report.Replace(" ", string.Empty).ToLower();
    <input class="ckbx leftmargin8" id="ckbx_@(morphedRptName)" type="checkbox" value="@rpt.report" />@rpt.report
}

`$('#all, #selectors').filter(':checked').first()` may get you somewhere. — amphetamachine, Apr 26 '16 at 16:16
What does the HTML look like? While we can improve things without changing it, we can probably improve them a lot more if we *do* change it. — T.J. Crowder, Apr 26 '16 at 16:18
@amphetamachine: That is **so much better** than my answer below. You should post it. I should hang my head in embarrassment. ;-) — T.J. Crowder, Apr 26 '16 at 16:18
@amphetamachine While simple, that doesn't handle the error if none are checked. — Don Rhummy, Apr 26 '16 at 16:23
@DonRhummy I'm not seeing any validation check on `rptval` in the OP. — amphetamachine, Apr 26 '16 at 16:25
It doesn't get to here if no checkbox is selected; I'll add that bit of code to the post to show that. — B. Clay Shannon-B. Crow Raven, Apr 26 '16 at 16:31
Why don't you give them all a common class? Then you can do `$(".class:checked").val()` to get the one that's checked? — Barmar, Apr 26 '16 at 16:39
I've always rised against huge of class just for `fun` (where not absolutely needed). Since class need to be in a stylesheet or is suppose to... Meanwhile its scope of usage is just a template or a page. — Salathiel Genese, Apr 26 '16 at 16:43

amphetamachine · Answer 1 · 2016-04-26T16:27:06.973

1

JQuery collections are filterable, so I'd suggest using the following:

// get value of first checked checkbox
var rptval = $(
   "#ckbx_produceusage,"+
   "#ckbx_deliveryperformance,"+
   "#ckbx_fillrate,"+
   "#ckbx_pricecompliance"
).filter(':checked').first().val();

Note: The .first() may even be unnecessary since .val() gets the value of the first element in the collection. Left in for legibility's sake.

edited Apr 26 '16 at 16:27

answered Apr 26 '16 at 16:21

amphetamachine

27,620
12
60
72

score 1 · Accepted Answer · answered Apr 26 '16 at 16:32

1

$(function()
{
...
// IF (these are the only elements that id starts with ckbx_) THEN
    rptval = $('[id^=ckbx_]').filter(':checked').val();
// ELSE
    // this syntax is more maintainable than $('#ckbx_produceusage, #ckbx_fillrate, ... selectors à la queue');
    rptval = $('#ckbx_produceusage').add('#ckbx_fillrate').add('#ckbx_deliveryperformance').add('#ckbx_pricecompliance').filter(':checked').val();
// FI
...
});

answered Apr 26 '16 at 16:32

Salathiel Genese

1,639
2
21
37

*"this syntax is more maintainable than..." Not even remotely, in my view. – T.J. Crowder Apr 26 '16 at 16:37
The first line is what I used, so I'm not worried about the "else" – B. Clay Shannon-B. Crow Raven Apr 26 '16 at 16:45
Thanks, but it might help someone else – Salathiel Genese Apr 26 '16 at 16:47
@T.J.Crowder, two years back, I realize I never understood your comment when you said **Not even remotely** – Salathiel Genese Oct 26 '18 at 12:42

score 0 · Answer 3 · answered Apr 26 '16 at 16:17

Without changing your HTML, you can at least avoid repeating yourself by using a loop:

["#ckbx_produceusage", "#ckbx_deliveryperformance", "#ckbx_fillrate", "#ckbx_pricecompliance"].some(function(sel) {
    var e = $(sel);
    if (e.is(":checked")) {
        rptval = e.val();
        return true;
    }
});
setEmailAndGenerateValsForUnitReportPair(unitval, rptval)

You could probably change your HTML such that the initial array could be a query rather than a hardcoded list of ID selectors.

score 0 · Answer 4 · answered Apr 26 '16 at 16:19

0

You also need to account for the situation where none are checked. Never assume something is set.

function getValue(checkBoxes)
{
    for ( var i = 0; i < checkBoxes.length; i++ )
    {
        var name = "#ckbx_" + checkBoxes[ i ];

        if ( $( name ).is( "checked" ) ) return $( name ).val();
    }

    //None were checked
    return null; //Should throw exception
}

rptval = getValue( [ "produceusage", "deliveryperformance", "fillrate", "pricecompliance" ] );

answered Apr 26 '16 at 16:19

Don Rhummy

24,730
42
175
330

id declare a variable `length` and assign it to `checkboxes.length` and add the new variable in the for loop so the length isn't calculated every time – Abdul Ahmad Apr 26 '16 at 16:32
@AbdulAhmad You're misunderstanding how that works. The length is not compiled every time through the loop. – Don Rhummy Apr 26 '16 at 17:22
i said calculated not compiled. but i guess its not recalculated every time according to some other questions – Abdul Ahmad Apr 26 '16 at 17:25

score 0 · Answer 5 · answered Apr 26 '16 at 16:32

0

Try using .each()

  $("input[type='checkbox']").each(function() {
    var rptval = 0;
    if ($(this).is(":checked")) {
      rptval = 1;
    }
    setEmailAndGenerateValsForUnitReportPair(unitval, rptval);
  });

answered Apr 26 '16 at 16:32

johnniebenson

540
3
9

This calls the function once for each checkbox, which is different from what OP's code does. – amphetamachine Apr 26 '16 at 17:53

How can I elegantize this verbose jquery?

UPDATE

5 Answers5

Linked