tags:

views:

41

answers:

3

I need help to consolidate/minify the code below. Originally I had a dropdown which controls div's visibility based on value. It works onClick but since it's a database page, it doesn't work onLoad so I had to add a 2nd logic. Any suggestions/input is greatly appreciated.

$(document).ready(function () {
  var $caseStatus = $(this).find("#auth_status").val();
  //alert ($caseStatus);
  if ($caseStatus === "1") {
    $(".pendingClass").show();
    $(".authClass").hide();
  }
  else if ($caseStatus === "2") {
    $(".pendingClass").hide();
    $(".authClass").show();
  }
  else {
    $(".pendingClass").hide();
    $(".authClass").hide();
  }

  // Show/Hide Divs for Authorization Status
  $("#auth_status").click(function () {
    if ($(this).val() === "0") {
      $(".pendingClass").hide();
      $(".authClass").hide();
    }
    else if ($(this).val() === "1") {
      $(".pendingClass").show();
      $(".authClass").hide();
    }
    else if ($(this).val() === "2") {
      $(".pendingClass").hide();
      $(".authClass").show();
    }
  });
});
+1  A: 

You could reduce your code a bit by simplifying your if/else structure(since anything besides "1" is the same show/hide result and also just call the handler you're binding when the page first loads, like this:

$(function () {
  $("#auth_status").click(function () {
    var val = $(this).val();
    $(".pendingClass").toggle(val === "1");
    $(".authClass").toggle(val === "2");
  }).click();
});
Nick Craver
+1 That's more elegant than my version. But the `$caseStatus` should be named `caseStatus`, actually. ;-) And the `$(this).val() === "1";` assumption is too simplified, since there are three cases.
Tomalak
@Tomalak - Better? :) I'm not sure what `1` translates to for statuses, but it should be named that status really, to be as clear as possible.
Nick Craver
@Tomalak - Woops, I read that `0` and `2` were the same, you're right those show/hides are different, updated with the correction.
Nick Craver
@Nick Craver: That's it. The thought to express it as `click(function …).click()` occurred to me a little too late, and now I don't want to change my answer to avoid looking like a copy cat. ;-)
Tomalak
@Tomalak - fair enough, +1 from me though :)
Nick Craver
Hm, that makes it… merely 150 points to go for the rep cap today! ;-)
Tomalak
@user486695 - sure!, all it does is fire that `click` handler on the element you just bound a `click` handler to, so we're re-using the same logic :)
Nick Craver
@Nick. Thanks man. Congrats btw :)
@user486695 - ty :)
Nick Craver
+1  A: 
$(function () {
  function toggleElements() {
    var caseStatus = $("#auth_status").val();
    $(".pendingClass").toggle(caseStatus === "1");
    $(".authClass").toggle(caseStatus === "2");
  };

  toggleElements();                         // initial call (on page load)
  $("#auth_status").click(toggleElements);  // manual call (on user click)
});
Tomalak
A: 

You could define a couple of objects:

var authCtrl = {
  '0': { show: null, hide: '.pendingClass, .authClass' },
  '1': { show: '.pendingClass', hide: '.authClass' },
  '2': { show: '.authClass', hide: '.pendingClass' }
};

and similarly (backwards) for the other case. Then the handler just grabs the entry based on that field value and hides/shows as directed.

Pointy
Isn't this a bit overkill? :)
Nick Craver
Well it depends. Encoding the behavior as data like this decouples the "rules" from the logic that acts on the rules. A handler written to interpret plain data like that (maybe even more generalized) can potentially be used for many similar structures that differ only in some particulars. I'm trying to teach myself Category Theory so I'm lately in the habit of thinking like this :-)
Pointy
@Nick: It would enable you to do `$(#auth_status").change(function () { $.each(authCtrl[$(this).val()], function(k, v) { $(v)[k](); }); });` :-D *(But only when all possible values of `#auth_status` are in `authCtrl`…)*
Tomalak
@Tomalak if the combinatorial problem gets big, then a much better approach would be to adjust the markup to be more cooperative.
Pointy
One approach would be to encode the "states" into the "behavior class" objects, instead of using just "show" and "hide". Since I have no clue how the page actually works, of course, it's hard to come up with the best answer.
Pointy
@Pointy: While I'm with you regarding the "ajust page markup" argument, I also think that things should be kept simple. My fun function two comments up shows that you can always make things more abstract, but maintainability suffers. And for in-browser code that shows of hides a few elements based on the state of a control, I'd tend to make it less clever and more obvious. :-)
Tomalak
Well @Tomalak *chacun à son goût* - YAGNI decisions are always hard to make and always subjective - overall I'd prefer a more completely unobtrusive mechanism anyway
Pointy
Which would be? Just being curious. :)
Tomalak
Well I'd say something that keeps the application semantics separate from the page behavior semantics, so that some generic Javascript would know to apply selected behaviors to the page. That way any page that has any sort of show/hide behavior just uses classes or data attributes to guide the code as to the element relationships. That way, new pages/forms don't need new Javascript.
Pointy