HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMajor

Usage of the ternary "?:" operator with functions listening to click events

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
theclickoperatoreventswithternaryusagelisteningfunctions

Problem

I've recently been doing some mods to some old code I've been maintaining for a couple of years now.

As part of a wider set of scripts using YAHOO YUI 2.2 (yes, that old) for dialog-style panels, I have a function that listens to click events on 3 buttons in a given panel set:

addFooListeners = function (panelType) {

    YAHOO.util.Event.addListener("show" + panelType, "click", showFoo, eval(ns + ".panel_" + panelType), true);
    YAHOO.util.Event.addListener("hide" + panelType, "click", hideFoo, eval(ns + ".panel_" + panelType), true);
    YAHOO.util.Event.addListener("commit" + panelType, "click", commitFoo, eval(ns + ".panel_" + panelType), true);

}


This code is the result of a number of panels/dialogs having near identical behaviour.

For this round of development I've needed to add the ability to do things a little differently from time to time, so I added an object, overrides which allows me to point the show, hide and commit actions elsewhere. I came up with the following:

addFooListeners = function (panelType, overrides) {

    var handlers = {
        show: ( overrides != null ? ( overrides.show != null ? overrides.show : showFoo ) : showFoo ),
        hide: ( overrides != null ? ( overrides.hide != null ? overrides.hide : hideFoo ) : hideFoo ),
        commit: ( overrides != null ? ( overrides.commit != null ? overrides.commit : commitFoo ) : commitFoo )
    }

    YAHOO.util.Event.addListener("show" + panelType, "click", handlers.show, eval(ns + ".panel_" + panelType), true);
    YAHOO.util.Event.addListener("hide" + panelType, "click", handlers.hide, eval(ns + ".panel_" + panelType), true);
    YAHOO.util.Event.addListener("commit" + panelType, "click", handlers.commit, eval(ns + ".panel_" + panelType), true);
}


As you can see I'm enforcing default behaviour unless an appropriate attribute in the overrides object is set with another function (named or anonymous).

Is there a cleaner / more readable way to concisely set

Solution

Let's do some fixing. First, this is how you pass optional (non-boolean) parameters in JS (the Good Waytm):

addFooListeners = function (panelType, handlers) {
    handlers        = handlers        || {};
    handlers.show   = handlers.show   || showFoo;
    handlers.hide   = handlers.hide   || hideFoo;
    handlers.commit = handlers.commit || commitFoo;


The above can be rewritten in a neater way using jQuery (not sure what the name of YUI equivalent to extend is):

handlers = $.extend({
    show  : showFoo, 
    hide  : hideFoo, 
    commit: commitFoo
}, handlers || {})


Now, using eval for this code is criminal. Say the object ns refers to is module, then you can do this instead of eval:

YAHOO.util.Event.addListener("show" + panelType, "click", handlers.show, module["panel_" + panelType], true);
YAHOO.util.Event.addListener("hide" + panelType, "click", handlers.hide, module["panel_" + panelType], true);
YAHOO.util.Event.addListener("commit" + panelType, "click", handlers.commit, module["panel_" + panelType], true);


Now, as you can see, you are assigning a lot of events in a similar fashion. Did you think of defining an addPanelListener function within your function?

function addPanelListener (event, panelType, handler) {
    YAHOO.util.Event.addListener(event + panelType, "click", handler, module["panel_" + panelType], true);
} 

addPanelListener("show"  , panelType, handlers.show);
addPanelListener("hide"  , panelType, handlers.hide);
addPanelListener("commit", panelType, handlers.commit):


Hope it helps.

Code Snippets

addFooListeners = function (panelType, handlers) {
    handlers        = handlers        || {};
    handlers.show   = handlers.show   || showFoo;
    handlers.hide   = handlers.hide   || hideFoo;
    handlers.commit = handlers.commit || commitFoo;
handlers = $.extend({
    show  : showFoo, 
    hide  : hideFoo, 
    commit: commitFoo
}, handlers || {})
YAHOO.util.Event.addListener("show" + panelType, "click", handlers.show, module["panel_" + panelType], true);
YAHOO.util.Event.addListener("hide" + panelType, "click", handlers.hide, module["panel_" + panelType], true);
YAHOO.util.Event.addListener("commit" + panelType, "click", handlers.commit, module["panel_" + panelType], true);
function addPanelListener (event, panelType, handler) {
    YAHOO.util.Event.addListener(event + panelType, "click", handler, module["panel_" + panelType], true);
} 

addPanelListener("show"  , panelType, handlers.show);
addPanelListener("hide"  , panelType, handlers.hide);
addPanelListener("commit", panelType, handlers.commit):

Context

StackExchange Code Review Q#472, answer score: 28

Revisions (0)

No revisions yet.