Explore our sites

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite

I have two different set of checkboxes. With the coffescript code below I set the maximum amount of checkable items at 3. I would like to refactor this code, to be cleaner and compact, but I can't get it.

$("div.feature_list :checkbox").click ->
  if $("div.feature_list :checked").length >= 3
    $("div.feature_list :checkbox:not(:checked)").attr "disabled", "disabled"
    $("div.feature_list :checkbox:not(:checked)").button disabled: true
  else
    $("div.feature_list :checkbox:not(:checked)").button disabled: false

$("div.style_list :checkbox").click ->
   if $("div.style_list :checked").length >= 3
     $("div.style_list :checkbox:not(:checked)").attr "disabled", "disabled"
     $("div.style_list :checkbox:not(:checked)").button disabled: true
   else
     $("div.style_list :checkbox:not(:checked)").button disabled: false
share|improve this question
1  
FYI, to enable/disable a button use .prop('disabled', true_or_false) instead of setting the attribute. –  ThiefMaster Oct 6 '12 at 19:17
add comment

migrated from stackoverflow.com Oct 6 '12 at 19:18

This question came from our site for professional and enthusiast programmers.

2 Answers

up vote 5 down vote accepted

Step 1: DRY. Encapsulate the "limit checkboxes" behavior in a function

limitCheckboxesIn = (container, limit = 3) ->
  checkboxes = $(container).find ":checkbox"
  checkboxes.on "click", (event) ->
    checked   = checkboxes.filter ":checked"
    unchecked = checkboxes.not checked
    state     = disabled: checked.length >= limit
    unchecked.prop(state).button state

Step 2: ... well, that pretty much it. Call it like so:

$ ->
  limitCheckboxesIn "div.feature_list"
  limitCheckboxesIn "div.style_list"

Here's a demo

By the way, if you just want the most compact solution, you can skip some assignments:

limitCheckboxesIn = (container, limit = 3) ->
  checkboxes = $(container).find ":checkbox"
  checkboxes.on "click", (event) ->
    disabled = checkboxes.filter(":checked").length >= limit
    checkboxes.not(":checked").prop({disabled}).button {disabled}

Personally, I find this a little less readable, but it's a matter of taste; the code's functionally identical.

share|improve this answer
 
It works with plain checkboxes but it doesnt work with jquery ui buttonset. I think the last line should be different –  framomo86 Oct 7 '12 at 0:29
 
@framomo86 Whoops, you're right - I've corrected my answer, and the demo. –  Flambino Oct 7 '12 at 10:33
add comment
up vote -1 down vote

I solved with this code

limitCheckboxesIn = (container, limit = 3) ->
  checkboxes = $(container).find ":checkbox"
  checkboxes.on "click", (event) ->
    checked   = checkboxes.filter ":checked"
    unchecked = checkboxes.not checked
    disable   = checked.length >= limit
    unchecked.prop "disabled", disable
    if disable
      unchecked.button disabled: true
    else
      unchecked.button disabled: false
share|improve this answer
 
I don't want to sound petty, but since your code is copied verbatim from my answer (except the very last part), you should not add your own answer here, but instead accept mine. The rule is basically to checkmark "the answer that helped the most". Yes, there was a bug in my code, but I fixed that as soon as I saw your comment about it. –  Flambino Oct 7 '12 at 22:04
 
-1, by the reasons @Flambino put out. (He's a good sport and didn't -1.) (But I also +1'd the question, so you get off on 4 positive rep... heh.) –  ANeves Oct 8 '12 at 18:13
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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