I needed to change properties from two objects from a array. So I did this
var tabs = [
{name: 'A', visible: false},
{name: 'B', visible: true},
{name: 'C', visible: true}}
];
var changeTabsVisibility = () {
if(validation()){
tabs.forEach(tab => {
switch(tab.name) {
case 'A':
tab.visible = true;
break;
case 'B':
tab.visible = false;
break;
default:
break;
}
});
}
}
It worked for what it was supposed to do, but was this a good pratice or the most efficient and comprehensive way to do this?
I think a clearer, less verbose, and less error-prone version would be to make an object mapping the tab names to their visibility.
const visibilityByTab = {
A: true,
B: false
}
tabs.forEach(tab => {
const newV = visibilityByTab[tab.name];
if (newV !== undefined) tab.visible = newV;
});
This is much more easily expandable to more tab names, and doesn't carry the possibility of introducing a bug if you ever happen to forget a break.
I think switch statement is not effeciant in this case.
It's simply converting name to visiblity, isn't it?
Your target is to convert name to visiblity, so it is only need a map which converts the name to visiblity
Here is an example what i thought.
var tabs = [
{name: 'A', visible: false},
{name: 'B', visible: true},
{name: 'C', visible: true}}
];
const NAME_TO_VISIBLE = {
'A': true,
'B': false,
'C': true,
...
};
var changeTabsVisibility = () {
if(validation()){
tabs.forEach(tab => {
tab.visible = NAME_TO_VISIBLE[tab.name] || false;
});
}
}