Reputation: 7163
Here's a bit of code which works fine but I think could be simplified / shortened. It's basically clicking on a list item, getting it's id and then showing / hiding / removing elements based on the id.
Suggestions on how to simplify this with a function or loop?
$("#btn_remove_event_type").click(function() {
var selectedId = $(".selected-type").attr("id");
if (selectedId == "temperature_event") {
$("#poplist_temp").show();
$(".temperature-params").hide();
$("#temperature_event").remove();
} else if (selectedId == "load_event") {
$("#poplist_load").show();
$(".load-params").hide();
$("#load_event").remove();
} else if (selectedId == "price_event") {
$("#poplist_price").show();
$(".price-params").hide();
$("#price_event").remove();
} else if (selectedId == "duty_event") {
$("#poplist_duty").show();
$(".duty-params").hide();
$("#duty_event").remove();
} else {
$("#poplist_program").show();
$(".program-params").hide();
$("#program_event").remove();
}
});
Upvotes: 1
Views: 70
Reputation: 7163
I ended up changing my classes to ids and reordering them, so I ended up with this solution which doesn't require any if statements.
$("#btn_remove_event_type").click(function() {
var selectedId = $(".selected-type").attr("id").split("_");
$("#" + selectedId[0] + "_pop").show();
$("#" + selectedId[0] + "_params").hide();
$("#" + selectedId[0] + "_event").remove();
});
Upvotes: 1
Reputation: 1586
Personally the cleanest and most configurable way i can see is using a map and looping of the methods. This is generic as different mappings can have different methods. You can even extend this further so you can have a method which will construct the mappings for you.. i.e. addNewType([{methodName:selector}, etc..]).
var configuration = {
temperature_event: {
show: "#poplist_temp",
hide: ".temperature-params",
remove: "#temperature_event"
},
load_event: {
show: "#poplist_load",
hide: ".load-params",
remove: "#load_event"
}
// etc etc.
};
$("#btn_remove_event_type").click(function() {
var fn, selector, typeConfig = $(".selected-type").attr("id");
for (fn in typeConfig)
$(typeConfig[fn])[fn]();
});
});
Upvotes: 0
Reputation: 7377
If you don't want to rely on the ID names matching other element names, you can use an object as a map to specify actions.
//using a map lets you easily add or remove cases without duplicating code, and doesn't require you to use standardized names for your elements and events.
var action_map = {temperature_event:{show:"#poplist_temp",
hide:".temperature-params",
remove:"#temperature_event"
},
load_event:{show:"#poplist_load",
hide:".load-params",
remove:"#load_event",
},
price_event:{show:"#poplist_price",
hide:".price-params",
remove:"#price_event",
},
duty_event:{show:"#poplist_duty",
hide:".duty-params",
remove:"#duty_event"
},
default_event:{show:"#poplist_program",
hide:".program-params",
remove:"#duty_event",
}
};
$("#btn_remove_event_type").click(function() {
var selectedId = $(".selected-type").attr("id");
if (!action_map[selectedId])
selectedId = "default_event";
$(action_map[selectedId].show).show();
$(action_map[selectedId].hide).hide();
$(action_map[selectedId].remove).remove();
}
});
Upvotes: 0
Reputation: 21
For simplicity sake I would change the id of the buttons clicked to one word(tempeature,load,price,duty). You'll have to do some renaming of your reactionary ids/classes, but just looks cleaner to me. I also flipped some of your naming around to stay consistent.
You can switch it around to match your style, it just bothered me having selectors like $("#"+id+"-params").hide();
$("#btn_remove_event_type").click(function() {
var selectedId = $(".selected-type").attr("id");
var nonDefault = ["temperature","load","price","duty"];
if(nonDefault.indexOf(selectedId) > -1){
$("#poplist_"+selectedId).show();
$(".params-"+selectedId).hide();
$("#event_"+selectedId).remove();
} else {
$("#poplist_program").show();
$(".params-program").hide();
$("#event_program").remove();
}
});
Upvotes: 1
Reputation: 19759
Something like this:
$("#btn_remove_event_type").click(function() {
var selectedId = $(".selected-type").attr("id");
switch(selectedId){
case "temperature_event":
$("#poplist_temp").show();
$(".temperature-params").hide();
$("#temperature_event").remove();
break;
case "load_event":
$("#poplist_load").show();
$(".load-params").hide();
$("#load_event").remove();
break;
case "price_event":
$("#poplist_price").show();
$(".price-params").hide();
$("#price_event").remove();
break;
case "duty_event"):
$("#poplist_duty").show();
$(".duty-params").hide();
$("#duty_event").remove();
break;
default:
$("#poplist_program").show();
$(".program-params").hide();
$("#program_event").remove();
}
});
Or even better:
$("#btn_remove_event_type").click(function() {
// Get id and split on "_"
var selectedId = $(".selected-type").attr("id").split("_");
// Set array of accepted input
var options = ["temperature","load","price","duty"];
// Handle default
if(options.indexOf(selectedId[0]) == -1){
selectedId = ["program"];
}
$("#poplist_" + selectedId[0]).show();
$("."+selectedId[0] + "-params").hide();
$("#"+selectedId[0] + "_event").remove();
});
Upvotes: 1