Reputation: 190
I'm trying to optimize my code to be more efficient and easier to read. I have some combined if-statements, which I think could be better, if they are transformed to for-loops, I'm just not sure how to do this?
This is my code:
if (starportSelected){
if(game.currentLevel.requirements.vehicles.indexOf('transport')>-1 && cashBalance>=vehicles.list["transport"].cost){
$("#transportbutton").removeAttr("disabled");
}
if(game.currentLevel.requirements.vehicles.indexOf('scout-tank')>-1 && cashBalance>=vehicles.list["scout-tank"].cost){
$("#scouttankbutton").removeAttr("disabled");
}
if(game.currentLevel.requirements.vehicles.indexOf('heavy-tank')>-1 &&cashBalance>=vehicles.list["heavy-tank"].cost){
$("#heavytankbutton").removeAttr("disabled");
}
if(game.currentLevel.requirements.vehicles.indexOf('harvester')>-1 && cashBalance>=vehicles.list["harvester"].cost){
$("#harvesterbutton").removeAttr("disabled");
}
if(game.currentLevel.requirements.aircraft.indexOf('chopper')>-1 && cashBalance>=aircraft.list["chopper"].cost){
$("#chopperbutton").removeAttr("disabled");
}
if(game.currentLevel.requirements.aircraft.indexOf('wraith')>-1 && cashBalance>=aircraft.list["wraith"].cost){
$("#wraithbutton").removeAttr("disabled");
}
}
I think first step would be to create two arrays, one for vehicles and one for aircrafts like this:
var vehicles = ['transport', 'scout.tank', 'heavy-tank', 'harvester'];
var aircraft = ['chopper', 'wraith'];
But how to take the rest of the code and change it to for-loop seems like a hard case for me. All help and explanation would be highly appreciated!
Upvotes: 2
Views: 107
Reputation: 2500
Seems that you have "vehicles"
and "aircraft"
types, with multiple values for each.
As such, I'd create an object of types to arrays of values.
Because you're also using a variable named vehicles
and aircraft
, you'll want to reference those in a separate object so that you can look them up with a string.
var lists = {
vehicles: vehicles,
aircraft: aircraft
}
var obj = {
vehicles: ["transport", "scout-tank", "heavy-tank", "harvester"],
aircraft: ["chopper", "wraith"]
};
Then use an outer and inner loop.
// v---("vehicles" or "aircraft")
for (var type in obj) { // v---("transport", "scout-tank", "chopper", etc...)
obj[type].forEach(function(val) {
if(game.currentLevel.requirements[type].indexOf(val)>-1 &&
cashBalance >= lists[type].list[val].cost) {
$("#" + val.replace("-", "") + "button").removeAttr("disabled");
}
});
}
Notice also that I had to replace the hyphen in the ID selector, since it wasn't used as part of the ID.
The two objects at the top could be combined into a single one if you wish:
var obj = {
vehicles: {
list: vehicles,
values: ["transport", "scout-tank", "heavy-tank", "harvester"]
},
aircraft: {
list: aircraft,
values: ["chopper", "wraith"]
}
};
Then adjust the loop references accordingly.
I've also cached the objects for performance, as Jared noted above.
for (var type in obj) {
var list = obj[type].list;
var requirements = game.currentLevel.requirements[type];
obj[type].values.forEach(function(val) {
if(requirements.indexOf(val)>-1 && cashBalance >= list[val].cost) {
$("#" + val.replace("-", "") + "button").removeAttr("disabled");
}
});
}
To make it even more efficient than the original, we'll drop some of the jQuery calls.
for (var type in obj) {
var list = obj[type].list;
var requirements = game.currentLevel.requirements[type];
for (var i = 0, vals = obj[type].values; i < vals.length; i++) {
var val = vals[i];
if(requirements.indexOf(val) > -1 && cashBalance >= list[val].cost) {
document.getElementById(val.replace("-", "") + "button").disabled = false;
}
}
}
Upvotes: 3