I KISS (Keep it simple, Stupid) but can this JavaScript structure be improved?

Question

I have done the following:

Put all event handlers inside jQuery DOM ready to ensure all events will only run
Define var selector on top of events for caching

Please see comments below for what I consider to be the problem

$(document).ready(){
    var searchCountry1 = $("#searchCountry1");
    var linkCountry1 = $("#linkCountry1");
    var codeCountry1 = $("[name='codeCountry1']");

    //worst part here is if I have 10 searches,links, and codes. is it worst?
    // and for every event I have this :
    searchCountry1.click(function(){
    //get json or open another search window
    });

    codeCountry1.keypress(function() { 
            codeCountry1.val(""); 
        //or clear all other related fields(city or state) to reset
   });

    $linkCountry1.click(function(){});
            //call redirect
    });

    function redirect(codeval,url){}

I've posted a small amount of code, but please help me imagine what if I have too many searches, fields and links having the same functionality but diff parameter?

I'm thinking about OOP or a design pattern for this. But no way, (I think for now). Please encourage me or suggest other ways on how to improve this code structure.

<input name="codeCountry1" /><br />
<input type="button" id="searchCountry1" /><br />
<a id="linkCountry1" href="#"><a/><br />
<input name="codeState1" /><br />
<input type="button" id="searchState1" /><br />
<a id="linkState1" href="#"><a/><br />
<input name="codeCity1" /><br />
<input type="button" id="searchCity1" /><br />
<a id="linkCity1" href="#"><a/><br />
<input name="codeOther1" /><br />
<input type="button" id="searchOther1" /><br />
<a id="linkOther1" href="#"><a/><br />
<input name="codeOther2" /><br />
<input type="button" id="searchOther2" /><br />
<a id="linkOther2" href="#"><a/><br />

Use DRY principles. Put a common class on all the `searchCountry`, `linkCountry` and `codeCountry` elements, then within the single event handler use the `this` keyword to reference the element that raised the event. You should also avoid nested event handlers where possible — Rory McCrossan, Aug 05 '16 at 08:54
@RoryMcCrossan thanks. I do avoid nested event handler sir. Sorry for the improper format above. Anyways, to clarify, If I have Country class, so I need another for City,State and other fields? — bumbumpaw, Aug 05 '16 at 09:03
That would be correct. If you could give a basic sample of your HTML then we can show you exactly how this would work — Rory McCrossan, Aug 05 '16 at 09:11
@RoryMcCrossan please see this https://jsfiddle.net/7fedg45a/ — bumbumpaw, Aug 05 '16 at 09:24
@madalinivascu when referring other sites, it is often helpful to point that [cross-posting is frowned upon](http://meta.stackexchange.com/tags/cross-posting/info) — gnat, Aug 05 '16 at 09:47
@gnat If I ever changed my mind on where to post this question,maybe I delete it here if this is not the right stack site :)) — bumbumpaw, Aug 05 '16 at 09:51

score 3 · Answer 1 · edited Jun 20 '20 at 09:12

It is obvious, that you need to use the same handler(s) on top of all your children that generate these events.

One of the most useful feature in browser is Event Bubbling

I prefer do not use classes for something that is not connected with styling. I use data-attributes.

So you need to wrap your children (if it is not wrapped yet), and give each children some data attribute that shows what handler to use:

<div data-role="wrapper">
  <input name="codeCountry1" data-action="clear" data-role="country"/><br />
  <input type="button" id="searchCountry1" data-action="search" data-role="country" /><br />
  <a id="linkCountry1" href="#" data-action="redirect" data-role="country"><a/><br />
  <input name="codeState1" /><br />
  <input type="button" id="searchState1" /><br />
  <a id="linkState1" href="#"><a/><br />
  <input name="codeCity1" /><br />
  <input type="button" id="searchCity1" /><br />
  <a id="linkCity1" href="#"><a/><br />
  <input name="codeOther1" /><br />
  <input type="button" id="searchOther1" /><br />
  <a id="linkOther1" href="#"><a/><br />
  <input name="codeOther2" /><br />
  <input type="button" id="searchOther2" /><br />
  <a id="linkOther2" href="#"><a/><br />
</div>

And listen to the children events on the wrapper:

var $wrapper = $( '[data-role="wrapper"]' );
$wrapper.on('click', '[data-action="search"][data-role="country"]', function(e) {
  console.log(e.target) // child element
  //get json or open another search window
});
$wrapper.on('click', '[data-action="redirect"][data-role="country"]', function(e) {
  console.log(e.target) // child element
  //call redirect
});
$wrapper.on('click', '[data-action="clear"][data-role="country"]', function(e) {
  console.log(e.target) // child element
  // from current element got to the wrapper and find all children inside it with something that you need
  var $other = $(e.target).closest('[data-role="wrapper"]').find('[data-action="clear"]').val("");
  //or clear all other related fields(city or state) to reset
});

This approach gives you not only three handlers for all children, you can also dynamically add or remove children without worrying about handlers. You can also remove id's from all elements (if you don't need form serialize).

As a disadvantage you see that you have to add logic layer into HTML, however it becomes more obvious in HTML what will be on click or something else...

whatt... are classes bad If I use it for the purpose of finding element? — bumbumpaw, Aug 10 '16 at 03:24
I haven't told you that classes are bad. But all my three years experience with SinglePageApplications taught me do not mix style and logic. — Alexey Vol, Aug 10 '16 at 12:07

score 1 · Answer 2 · answered Aug 15 '16 at 00:10

If you want to keep logical structures off your markup or can‘t or don‘t want to modify it you can still use arrays to improve readability. Just gather all your selectors in an array and operate on that array:

jQuery(function($) {

    var triplets = [
        [ "#searchCountry1", "linkCountry1", "[name='codeCountry1']" ],
        [ "#searchCountry2", "linkCountry2", "[name='codeCountry2']" ],
        [ "#searchCountry3", "linkCountry3", "[name='codeCountry3']" ]
    ];

    triplets.forEach(function(triplet) {
        var search = $(triplet[0]);
        var link = $(triplet[1]);
        var code = $(triplet[2]);

        search.on("click", function(e) {
            // get json or open another search window
            // use link and/or code in here
        }
        code.on("click", function(e) {
            code.val("");
            // or clear all other related fields(city or state) to reset
        }
        link.on("click", function(e) {
            // call redirect
        });
    });

    function redirect(codeval, url) {
        // …
    }
});

If you don‘t mind touching your markup head over for alexey‘s answer and gain maintainability and performance.

I KISS (Keep it simple, Stupid) but can this JavaScript structure be improved?

2 Answers2