0

I have adopted some jQuery code that has like 20 event-captures that look like this

  // used on menu#edit
  $('.show-headers-only-toggle').on('click',function(){
    arc_eh.menu_edit.show_headers_only_toggle.call(this);
  });

that call a method that does the functionality. This is the only place that this method is called. I want to put the functionality after the event like this since I think there's no reason to have this other method. The argument for having it in a method is that I'm polluting this space and the event handling is like a router and this is a controller. I feel like this is bikeshedding and we're not using a framework so it's just simpler to have all in place. Who do you feel is wrong in this? I feel they are bikeshedding this.

  // used on menu#edit
  $('.show-headers-only-toggle').on('click',function(){
    var action=$(this).attr('arc-action');
    // this would just be what is in the function above
    if(action=='hide-items'){
      $('.menu-item-admin').hide();
      $(this).text('show items');
      $(this).attr('arc-action','show-items');
    }else{
      $(this).text('show headers only');
      $('.menu-item-admin').show();
      $(this).attr('arc-action','hide-items');
    }
  });
1

I greatly prefer the first chunk of code. The fact that it is only used once doesn't matter. The great thing about small methods is that you can show intentions of the code without having to read it. You might want to check out something like Clean Code, the early chapters really talk about the importance of well named methods.

$('.show-headers-only-toggle').on('click',function(){
    arc_eh.menu_edit.show_headers_only_toggle.call(this);
  })

Without knowing anything of your code base I can assume that the following method toggles the headers of an edit menu. Looking at the other chunk of code I really can't tell what it is doing without reading the code.

There are also other benefits too. What happens when you want to have that functionality outside of just a mouse click for example in a hot key? Also how are you going to test that the method works? There are frameworks that can fire events on dom elements but that can add extra bloat and event knowledge to your tests.

| improve this answer | |
  • thx, I apprecaite the feedback. I had a hand in creating this so I'm not fully against it but I'm starting to loathe the unnecessary indirection that is explicit in this. Part of this is also the nature of Javascript. If this were coupled to a library, I think I'd be better with it but it's somewhat happenstance. Thinking about ordering clean code. Have you read Code Complete and have an opinion on which you prefer? – timpone Jan 28 '14 at 22:57
  • I haven't read Code Complete but I do recommend Clean Code. I can see where you are coming from with the indirection but I think things would make more sense if they were wrapped in some event binding framework. – pllee Jan 28 '14 at 23:07
  • agreed with your thinking (I ordered a copy from Code Complete) and I did advocate this structure originally. However, going back to it, I find it somewhat unpleasant having code spread across 4 places for this type of jQuery work for what's really like 15 or 20 lines of code. I get seperation of concerns but I think just overkill in this situation. – timpone Jan 29 '14 at 16:09
  • Also, you mention an 'event binding framework' - do you mean that jQuery is or isn't this? – timpone Jan 29 '14 at 16:26
  • jQuery is dom normalization/manipulation framework. There are other frameworks that are build on top of jQuery that handle the binding of ui/model events to views/controllers. Such as canjs and backbonejs and many other frameworks. – pllee Jan 29 '14 at 16:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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