1
\$\begingroup\$

I have the following series of click events in my jQuery code:

$('#morning .up .hour').click(function () {
  morning.hour++
  update_hour(morning);
});

$('#morning .up .min').click(function () {
  morning.min++
  update_min(morning);
});

$('#morning .up .sec').click(function () {
  morning.sec++
  update_sec(morning);
});

$('#morning .down .hour').click(function () {
  morning.hour--
  update_hour(morning);
});

// ...
// morning down min
// morning down sec
//
// evening up hour
// evening up min
// evening up sec
//
// evening down hour
// evening down min
// ...

$('#evening .down .sec').click(function () {
  evening.sec--
  update_sec(evening);
});

Each alarm has an object which contains the values for the hours, minutes and seconds, along with the selector for the values (this is used in the update functions explained below):

var morning = {
  selecthour: "#morning .time .hour",
  selectmin: "#morning .time .min",
  selectsec: "#morning .time .sec",
  hour: 7,
  min: 0,
  sec: 0
};

And finally the HTML code:

<div id="morning">
    <div class="wrapper"> <!-- Wrapper for styling -->
        <div class="up"> <!-- Up arrows -->
            <div class="hour">Up</div>
            <div class="min">Up</div>
            <div class="sec">Up</div>
        </div>
        <div class="time"> <!-- hour, min and sec fields -->
            <span class="hour">EX</span>:<span class="min">AM</span>:<span class="sec">PL</span>
        </div>
        <div class="down"> <!-- Down arrows -->
            <div class="hour">Down</div>
            <div class="min">Down</div>
            <div class="sec">Down</div>
        </div>
    </div>     
</div>

The basic structure of the page is:

  • There are several alarms, each of which has a unique ID (currently there are two: one named morning, one named evening.)
  • Each alarm has an hour, minute and second field (see div.time)
  • Each field, in each alarm, has an up and a down arrow which increases or decreases the time
  • In the selectors above, the ID is the alarm, the first class is the up or down arrow, and the second class is the field which the arrow is associated with (hour/min/sec)

When a button is clicked, the following should happen:

  • The hour, min or sec property of the alarm object (morning or evening) is incremented or decremented by 1, based on the arrow pressed
  • The update_hour, update_min, or update_sec function is called, with the alarm object as its argument (morning or evening)
  • The update function checks and corrects any invalid input, then updates the appropriate field of the appropriate alarm in the HTML.

It takes 6 functions, each 3 lines long, for every alarm. For just the two alarms that's 36 lines of repetitive code.

Is it possible to simplify this series of jQuery .click() statements? Bonus points if it works with more than the two specified alarms. (I have an array containing all of the alarms if that helps)

\$\endgroup\$

migrated from stackoverflow.com Jun 17 '16 at 1:28

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

1
\$\begingroup\$

Your name of properties and selectors has consistency, so make a arrays of the parts that change.

I've also added the objects you're mutating and the functions you're calling to other objects so that they can be referenced using a variable.

var am_pms = ["morning", "evening"]
var dirs = ["up", "down"]
var units = ["sec", "min", "hour"]

var am_pm_obj = {
  morning: morning,
  evening: evening
}
var update_funcs = {
  sec: update_sec,
  min: update_min,
  hour: update_hour
}

Then iterate the arrays, patching in the parts. This will cover all the functions you need to create:

am_pms.forEach(function(morn_even) {  
  dirs.forEach(function(dir) {
    var offset = dir === "up" ? 1 : -1

    units.forEach(function(s_m_h) {
      $('#' + morn_even + ' .' + dir + ' .' + s_m_h).click(function () {
        am_pm_obj[morn_even][s_m_h] += offset
        update_funcs[s_m_h](am_pm_obj[morn_even])
      })
    })
  })
})
  • morn_even will be "morning" or "evening"
  • dir will be "up" or "down"
  • offset will be 1 or -1, which replaces ++ and --
  • s_m_h will be "sec", "min" or "hour"

The string variables will be used to build the selector and also to access the objects you mutate (and pass) and the functions you call, which is why the objects and functions needed to be nested in another object.

\$\endgroup\$
1
\$\begingroup\$

Use data attributes on the DIVs to indicate which field should be updated.

    <div class="up"> <!-- Up arrows -->
        <div data-field="hour">Up</div>
        <div data-field="min">Up</div>
        <div data-field="sec">Up</div>
    </div>

Then use a single handler:

$("#morning .up div").click(function() {
    var field = $(this).data("field");
    morning[field]++;
    update_time(morning, field);
});

Instead of separate update_hour, update_min, update_sec functions, combine them into a single update_time function that takes the field to update as a parameter.

You could go further and combine all the handlers for #morning and #evening by adding data-timeperiod="morning/evening", and combine up/down with data-increment="1/-1".

\$\endgroup\$
  • \$\begingroup\$ I can't combine the update functions, since each of the hour, minute and second fields require different validation/input checking, and a combined function would make the code multiple times longer. \$\endgroup\$ – angussidney Jun 11 '16 at 12:08
  • \$\begingroup\$ Maybe you shouldn't use an object with separate hour/minute/second fields, but just a single time value that you add 1, 60, or 3600 to. Then you convert this to hour:minutes:seconds when displaying it. \$\endgroup\$ – Barmar Jun 11 '16 at 12:12
  • \$\begingroup\$ Good point - I should look into that. It might also solve something else I was trying to do. However it might require a major re-write :/ \$\endgroup\$ – angussidney Jun 11 '16 at 12:13
  • \$\begingroup\$ There's no free lunch. \$\endgroup\$ – Barmar Jun 11 '16 at 12:14
  • \$\begingroup\$ Another idea: var timefield_limits = { hours: 24, minutes: 60, seconds: 60 }. Use this in the update_time function so it can compare the named field against timefield_limits[field]. \$\endgroup\$ – Barmar Jun 11 '16 at 12:16
0
\$\begingroup\$

I would use below HTML

<div class="hour" data-type="morning-hour-up">Up</div>

And then write javascript accordingly

$('[data-type]').click(function () {
  var dataTypes = $(this).data("type").split('-');
  // for morning-hour-up
  // dataTypes[0] will be morning
  // dataTypes[2] will be hour 
  // dataTypes[2] will be up

  // for incremental and decremental
  if(dataTypes[2] == "up") {
    window[dataTypes[0]][dataTypes[1]]++;
  } else {
    window[dataTypes[0]][dataTypes[1]]--;
  }

  // For calling functions
  window["update_" + dataTypes[1]].call(this, dataTypes[0]);
});

NOTE: Here I am using window variable and putting everything under window scope, but you can have different global variable which can be accessed easily

Let me know if there is unclarity about my answer...

\$\endgroup\$
0
\$\begingroup\$

@squint nice way of using the functions and variables. But as mentioned am_pm can more, not just ['morning', 'evening']. One small correction to the function

am_pm.forEach(function(p1, i) {
    unit.forEach(function(p2) {
        $('#' + p1 + ' .' + p2).click(function () {
            if ($(this).hasClass("up")){
                am_pm_obj[p1][p2] += 1
            }else{
                am_pm_obj[p1][p2] += -1
            }
            update_funcs[p1](am_pm_obj[p1]);
        });
    })
})
\$\endgroup\$

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.