Function to toggle checkboxes

Question

I have a form with two date (in format "YYYY-MM-DD") input fields 'Invoice sent'/'Payment received' and each of them has two checkboxes "pending"/"obsolete".

If one of the checkboxes is clicked, then the input field should be filled with '1111-11-11' or '0000-00-00' and the sibling checkboxes should be disabled.

This code works so far and does exactly that.

$(document).ready(function(){
  $("input.invoice-obsolete").click(toggle_checkbox);
  $("input.invoice-pending").click(toggle_checkbox);
  $("input.payment-obsolete").click(toggle_checkbox);
  $("input.payment-pending").click(toggle_checkbox);
  var invoice_sent = $(".invoice-sent").val();
  var payment_received = $(".payment-received").val();
  if (invoice_sent == '1111-11-11') {
  $("input.invoice-obsolete").attr("checked",true);
  }
  if (invoice_sent == '0000-00-00') {
    $("input.invoice-pending").attr("checked",true);
  }
  if (payment_received == '1111-11-11') {
    $("input.payment-obsolete").attr("checked",true);
  }
  if (payment_received == '0000-00-00') {
    $("input.payment-pending").attr("checked",true);
  }
});

function toggle_checkbox() {
  if($('input.invoice-obsolete').prop('checked')) {
    $("input.invoice-pending").attr("disabled", true);
    $('input.invoice-sent').val('1111-11-11');
    $("input.invoice-sent").attr("disabled", true);
  };

  if($('input.invoice-pending').prop('checked')) {
    $("input.invoice-obsolete").attr("disabled", true);
    $('input.invoice-sent').val('0000-00-00');
    $("input.invoice-sent").attr("disabled", true);
  };

  if(! $('input.invoice-obsolete').prop('checked')) {
    $("input.invoice-pending").removeAttr("disabled");
  }

  if(! $('input.invoice-pending').prop('checked')) {
    $("input.invoice-obsolete").removeAttr("disabled");
  }

  if ( (! $("input.invoice-pending").is(':checked')) && (! $("input.invoice-obsolete").is(':checked')) ) {
    $("input.invoice-sent").attr("disabled", false);
  }

  if($('input.payment-obsolete').prop('checked')) {
    $("input.payment-pending").attr("disabled", true);
    $('input.payment-received').val('1111-11-11');
    $("input.payment-received").attr("disabled", true);
  };

  if($('input.payment-pending').prop('checked')) {
    $("input.payment-obsolete").attr("disabled", true);
    $('input.payment-received').val('0000-00-00');
    $("input.payment-received").attr("disabled", true);
  };

  if(! $('input.payment-obsolete').prop('checked')) {
    $("input.payment-pending").removeAttr("disabled");
  }

  if(! $('input.payment-pending').prop('checked')) {
    $("input.payment-obsolete").removeAttr("disabled");
  }

  if ( (! $("input.payment-pending").is(':checked')) && (! $("input.payment-obsolete").is(':checked')) ) {
    $("input.payment-received").attr("disabled", false);
  }
}

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>

<div>
  <label for="form-horizontal-text">Invoice sent</label>
  <input placeholder="YYYY-MM-DD" class="invoice-sent" type="text" name="job[invoice_sent]" id="job_invoice_sent" readonly="readonly">
  <div>
    <input name="job[invoice_sent_checkbox]" type="hidden" value="0"><input class="invoice-obsolete" type="checkbox" value="1" name="job[invoice_sent_checkbox]" id="job_invoice_sent_checkbox"> 
    <span class="invoice-sent">obsolete</span>
    <input name="job[invoice_sent_checkbox]" type="hidden" value="0"><input class="invoice-obsolete" type="checkbox" value="1" name="job[invoice_sent_checkbox]" id="job_invoice_sent_checkbox" disabled=""> 
    <span>pending</span>
  </div>
</div>

<div>
  <label for="form-horizontal-text">Payment received</label>
  <input placeholder="YYYY-MM-DD" class="payment-received" type="text" name="job[payment_received]" id="job_payment_received" readonly="readonly">
  <div>
    <input name="job[payment_received_checkbox]" type="hidden" value="0"><input class="payment-obsolete" type="checkbox" value="1" name="job[payment_received_checkbox]" id="job_payment_received_checkbox"> 
    <span class="invoice-sent">obsolete</span>
    <input name="job[payment_received_checkbox]" type="hidden" value="0"><input class="payment-pending" type="checkbox" value="1" name="job[payment_received_checkbox]" id="job_payment_received_checkbox"> 
    <span>pending</span>
  </div>
</div>

How can I refactor the code, so that I avoid unnecessary duplication?

Answer 1

There seem to be some bugs in the code:

• JS code mentions input.invoice-pending, but there is no such element.
• id attribute values should be unique on page.
• name attribute values should also be unique within a form.

Suggestions for refactoring:

• There are two parts with similar behavior: one for invoice, one for payment. Instead of prefixing each class name with invoice- or payment-, assign that class to the containing div and drop the prefix from elements inside:

  <div class="invoice">
      <input class="date" />
      <input class="obsolete" />
      <input class="pending" />
  </div>
  <div class="payment">
      <input class="date" />
      <input class="obsolete" />
      <input class="pending" />
  </div>
  

  Then you can create a generic function for handling both parts of the form:

  function initForm($container) {
      $container.find("input.obsolete").click(toggle_checkbox);
      ...
  }
  

  And then call it for both parts:

  $(document).ready(function(){
      initForm($("invoice"));
      initForm($("payment"));
  });
  

• Instead of always performing a query like $('input.invoice-pending'), you can store the result to a variable and then use the variable:

  // Use plural form to show that we select multiple elements
  var $obsoleteInvoices = $('input.invoice-obsolete');