How to make this statement more efficient?

Question

I want the same thing to happen for each of a series of radio buttons when they are checked. So far I have the following working code:

if ($("input[type=radio]:checked#rd1").length > 0) {
        // do something
  } else if ($("input[type=radio]:checked#rd2").length > 0) {
        // do something
  } else if ($("input[type=radio]:checked#rd3").length > 0) {
        // do something   
  }

I tried to do the following:

if ($("input[type=radio]:checked#rd1"||"input[type=radio]:checked#rd2").length > 0) {...}

and I got a warning to say it was 'heuristically unreachable'.

Anyway of making this more compact?

EDIT: If #rd1 / #rd2 / #rd3 are checked they all do the exact same thing. // do something in this case means the same outcome for each radio button. So I wanted to make the if statement more concise as it results in the same outcome for each 'checked'.

Because there is only one radio that can be checked per group, why don't you just bind change event and check for ID in handler? BTW, because IDs must be unique on document context, this e.g would be enough as selector: `#rd1:checked`. That's said, could you provide all relevant code in question itself as HTML markup and expected behaviour (the 'do something' part) to make your question clearer. EDIT: now i'm just wondering is the `// do something` the same for all??? — A. Wolff, Jul 08 '15 at 14:14
Why not `if ($("input[type=radio]:checked").length > 0) {` instead all your `if` statements. If you dont have anothers `input[type=radio]` should work. — lmgonzalves, Jul 08 '15 at 14:20
I agree with @A.Wolff . . . I can see a number of ways to potentially make this WAY more efficient, but we don't have enough information about exactly what you are trying to do to be able to give an appropriate answer. — talemyn, Jul 08 '15 at 14:20
possible duplicate of [Find out if radio button is checked with JQuery?](http://stackoverflow.com/questions/2272507/find-out-if-radio-button-is-checked-with-jquery) — Coding Enthusiast, Jul 08 '15 at 14:26

Coding Enthusiast · Accepted Answer · 2015-07-08T14:20:10.917

2

Judging from what you did before: this should fix what you tried.

if ($("input[type=radio]:checked#rd1").length > 0 
    ||$("input[type=radio]:checked#rd2").length > 0 || $("input[type=radio]:checked#rd3").length > 0) {
        // do something   
}

There are other ways to do this effectively in JQuery like using name.

if ($("input[name='name']").is(':checked')) {
   //do something
}

or through a classname

if($("input:radio[class='className']").is(":checked")) {
     //do something        
}

edited Jul 08 '15 at 14:20

answered Jul 08 '15 at 14:14

Coding Enthusiast

3,865
1
27
50

This worked perfectly, of course I needed to include the whole statement. Thank you! – Rebecca O'Riordan Jul 08 '15 at 14:17
2

@foreverlearning: This is different from your original code. It now does the same `// do something` for all 3 cases. You sure that's what you want? – Jul 08 '15 at 14:19
@foreverlearning Then this would be more 'compact': `$(':radio[id^=rd]:checked').length` BUT if it is really what you are looking for, your question doesn't really make sense... – A. Wolff Jul 08 '15 at 14:19
@squint judging from his question he is simply looking for ways to check if radio button is selected.That's why he tried to compact it into one . Think about his question. – Coding Enthusiast Jul 08 '15 at 14:24
Well, he said he wants to shorten the original code. So how it gets shortened hinges on what's in those `// do something` blocks. The original could run different code based on which radio is selected, which seems likely. Think about his question. – Jul 08 '15 at 14:27
@squint I was thinking the same but obvously OP is a beginner so finally question and this answer makes sense – A. Wolff Jul 08 '15 at 14:29
@squint my code does the same thing for all 3 cases so this solution was the right one. – Rebecca O'Riordan Jul 08 '15 at 14:33
@foreverlearning: Thanks for the update. FYI, in the future, it's a good idea to give a sample of the DOM by posting your markup. A good solution for DOM selection is heavily dependent on your document structure. – Jul 08 '15 at 14:35

Reinstate Monica Cellio · Answer 2 · 2015-07-08T15:03:49.150

There's no need for the lengthy selector since IDs have to be unique. Also, 0 is false in this scenario, so you could trim it down and just have this...

if ($("#rd1:checked").length) {
    // do something
}
else if ($("#rd2:checked").length) {
    // do something
}
else if ($("#rd2:checked").length) {
    // do something
}

Since you've now confirmed that // do something is the same in all scenarios, I'd change it to this...

if ($("#rd1:checked, #rd2:checked, #rd3:checked").length) {
    // do something
}

How to make this statement more efficient?

2 Answers2