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

Function which will fire off a specific template depending on which element is selected

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

Problem

I have this function which will fire off a specific template depending on which element is selected. However, with the way it is, I'd have to update the JavaScript and the HTML when a new element is added. What I'd like is to have the function scale up to as many HTML elements one would add.

So far I have figured out if I do this:

function templateLoader(e) {
        var doc = document,
            body = doc.getElementsByTagName('body'),
            event = EventUtility.getEvent(e),
            target = EventUtility.getTarget(event),
            overlay = $('#overlay');

        console.dir(event.currentTarget);

        if (target !== event.currentTarget) {

            switch(target.id){
                case '1':
               $.get('../../../templates/index.html',function(template) {
                 modal.append(Mustache.render($(template).filter('#templateID').html()))
                 modal.listview("refresh");
                 });
                break;

                case '2':
                alert(target.title);
                break;

                case '3':
                alert(target.title);
                break;

                case '4':
                alert(target.title);
                break;

                case '5':
                alert(target.title);
                break;
            }
        }

        EventUtility.stopPropagation(event);

    }

    EventUtility.addHandler(flimFlam, 'click', templateLoader);

});


HTML:


      
       

       

       

       


But of course that depends on me going int there and adding newer case conditions when needed. I know I'll have to go into the files anyway when the HTML is added. But I wanted a way to have the JS ready for any amount of cases.

I began with something like this:

for (var i = 0, l = event.target.length; i < l; i++) {
                console.log(target.id[i]);
            };

Solution

body = doc.getElementsByTagName('body'),


There's a shorter document.body. But then, body isn't even being used in the function. So you probably have to remove it.

console.dir(event.currentTarget);
alert(target.title);


I suggest you don't pepper your code with console.* and alert to view values. Take advantage of the browser's debugger, and use breakpoints. Not only can you view the variable, you can also view surrounding variables, enclosing scopes and more.

switch(target.id){


First, I wouldn't use id. id is supposed to be unique in the page. Use it if you're absolutely sure that the element is the only one in the page with that id. This can get real messy when you discover that your HTML needs to be in multiple places, but contain the same structure and/or content.

But then, I notice you use data-* attributes. Use that instead of id. It's designed for that purpose, carry data in markup.

case '1':
$.get('../../../templates/index.html',function(template) {
  modal.append(Mustache.render($(template).filter('#templateID').html()))
  modal.listview("refresh");
  });

 break;

 case '2':
 alert(target.title);
 break;

 case '3':
 alert(target.title);
 break;

 case '4':
 alert(target.title);
 break;

 case '5':
 alert(target.title);
 break;


It doesn't have a default. switch is also a bit clunky. And I usually avoid switch because failing to add a break where needed will cause your code to run down an unexpected path.

The following would be a better way to model it:

// An operations object, with key as the id, and function as value
var operations = {
  '1': function(){
    $.get('../../../templates/index.html',function(template) {
      modal.append(Mustache.render($(template).filter('#templateID').html()))
      modal.listview("refresh");
      });
  },
  '2': function(){
    alert(target.title);
  },
  // and so on
}

if(typeof operations[target.id] === 'function') operations[target.id].call(null);

Code Snippets

body = doc.getElementsByTagName('body'),
console.dir(event.currentTarget);
alert(target.title);
switch(target.id){
case '1':
$.get('../../../templates/index.html',function(template) {
  modal.append(Mustache.render($(template).filter('#templateID').html()))
  modal.listview("refresh");
  });

 break;

 case '2':
 alert(target.title);
 break;

 case '3':
 alert(target.title);
 break;

 case '4':
 alert(target.title);
 break;

 case '5':
 alert(target.title);
 break;
// An operations object, with key as the id, and function as value
var operations = {
  '1': function(){
    $.get('../../../templates/index.html',function(template) {
      modal.append(Mustache.render($(template).filter('#templateID').html()))
      modal.listview("refresh");
      });
  },
  '2': function(){
    alert(target.title);
  },
  // and so on
}

if(typeof operations[target.id] === 'function') operations[target.id].call(null);

Context

StackExchange Code Review Q#106767, answer score: 3

Revisions (0)

No revisions yet.