patternjavascriptMinor
UI for adding roles and modules to a user list
Viewed 0 times
modulesrolesuseraddingforandlist
Problem
I have a small UI that involves adding roles and modules to a user database array. The interface works as is, no problem at all, but it seems to have a bit of 'duplicate' code that I was hoping I could get some advice on ways to refactor, if at all possible.
JS and Knockout
```
function Db() {
var self = this;
self.userRoles = ['Executive', 'Admin', 'User'];
self.userModules = ['Explore', 'Dashboards', 'Alerts'];
self.userDatabases = ko.observableArray();
self.allRolesSelected = ko.observableArray();
self.allModulesSelected = ko.observableArray();
self.databases = _.range(5).map(function (i) {
return {
name: 'DB ' + (i + 1),
chosenRoles: ko.observableArray(),
chosenModules: ko.observableArray()
};
});
self.toggleAllRoles = function (r, e) {
var checked = e.target.checked;
if (checked) {
self.setAllRoles(r);
} else {
self.clearAllRoles(r);
}
};
self.toggleAllModules = function (r, e) {
var checked = e.target.checked;
if (checked) {
self.setAllModules(r);
} else {
self.clearAllModules(r);
}
};
self.setAllRoles = function (r) {
_.each(self.userDatabases(), function (d) {
if (d.chosenRoles.indexOf(r) === -1) {
d.chosenRoles.push(r);
}
});
};
self.setAllModules = function (r) {
_.each(self.userDatabases(), function (d) {
if (d.chosenModules.indexOf(r) === -1) {
d.chosenModules.push(r);
}
});
};
self.clearAllRoles = function (r) {
_.each(self.userDatabases(), function (d) {
d.chosenRoles.remove(r);
});
};
self.clearAllModules = function (r) {
_.each(self.userDatabases(), function (d) {
d.chosenModules.remove(r);
});
};
ko.computed(function () {
JS and Knockout
```
function Db() {
var self = this;
self.userRoles = ['Executive', 'Admin', 'User'];
self.userModules = ['Explore', 'Dashboards', 'Alerts'];
self.userDatabases = ko.observableArray();
self.allRolesSelected = ko.observableArray();
self.allModulesSelected = ko.observableArray();
self.databases = _.range(5).map(function (i) {
return {
name: 'DB ' + (i + 1),
chosenRoles: ko.observableArray(),
chosenModules: ko.observableArray()
};
});
self.toggleAllRoles = function (r, e) {
var checked = e.target.checked;
if (checked) {
self.setAllRoles(r);
} else {
self.clearAllRoles(r);
}
};
self.toggleAllModules = function (r, e) {
var checked = e.target.checked;
if (checked) {
self.setAllModules(r);
} else {
self.clearAllModules(r);
}
};
self.setAllRoles = function (r) {
_.each(self.userDatabases(), function (d) {
if (d.chosenRoles.indexOf(r) === -1) {
d.chosenRoles.push(r);
}
});
};
self.setAllModules = function (r) {
_.each(self.userDatabases(), function (d) {
if (d.chosenModules.indexOf(r) === -1) {
d.chosenModules.push(r);
}
});
};
self.clearAllRoles = function (r) {
_.each(self.userDatabases(), function (d) {
d.chosenRoles.remove(r);
});
};
self.clearAllModules = function (r) {
_.each(self.userDatabases(), function (d) {
d.chosenModules.remove(r);
});
};
ko.computed(function () {
Solution
Here are a few items that I could think of. I know nothing about Knockout so if these changes do not work in that framework (I don't know why they wouldn't).
The first thing I would do is wrap the code with an IIFE to create a private scope. Again, this might not be necessary with Knockout.
Next, you can DRY out your code quite a bit. For example, this:
could be re-written as this just by passing one additional variable:
You can do something similar with the
Lastly, I would again do this in the last section:
Unfortunately, I have no way to test this code.
I hope that helps!
The first thing I would do is wrap the code with an IIFE to create a private scope. Again, this might not be necessary with Knockout.
(function(){
//your code here
})();Next, you can DRY out your code quite a bit. For example, this:
self.toggleAllRoles = function (r, e) {
var checked = e.target.checked;
if (checked) {
self.setAllRoles(r);
} else {
self.clearAllRoles(r);
}
};
self.toggleAllModules = function (r, e) {
var checked = e.target.checked;
if (checked) {
self.setAllModules(r);
} else {
self.clearAllModules(r);
}
};could be re-written as this just by passing one additional variable:
self.toggleAll = function ( r, e, roleOrModule ) {
// roleOrModule = 'role' or 'module' ( or anything not 'role' )
self.setOrClear( r, roleOrModule, e.target.checked );
};You can do something similar with the
setAllRoles, setAllModules, clearAllRoles and clearAllModules methods:self.setOrClear = function( r, roleOrModule, pushOrRemove ) {
var method = ( roleOrModule === 'role' ) ? 'chosenRoles' : 'chosenModule';
var pushRemove = ( pushOrRemove ) ? 'push' : 'remove';
_.each( self.userDatabases(), function (d) {
if ( d[ method ].indexOf( r ) === -1) {
d[ method ][ pushRemove ](r);
}
});
};Lastly, I would again do this in the last section:
ko.computed(function () {
var eachMethod = ['userRoles','userModules'];
var chosenMethod = ['chosenRoles','chosenModules'];
var allMethod = ['allRolesSelected', 'allModulesSelected'];
for (var i=0, l = eachMethod.length; i 0 && _.every(self.userDatabases(), function (d) {
return d[ chosenMethod[i] ].indexOf(r) > -1;
});
if (all) {
if (self[ allMethod[i] ].indexOf(r) === -1) {
self[ allMethod[i] ].push(r);
}
} else {
self[ allMethod[i] ].remove(r);
}
});
}
});Unfortunately, I have no way to test this code.
I hope that helps!
Code Snippets
(function(){
//your code here
})();self.toggleAllRoles = function (r, e) {
var checked = e.target.checked;
if (checked) {
self.setAllRoles(r);
} else {
self.clearAllRoles(r);
}
};
self.toggleAllModules = function (r, e) {
var checked = e.target.checked;
if (checked) {
self.setAllModules(r);
} else {
self.clearAllModules(r);
}
};self.toggleAll = function ( r, e, roleOrModule ) {
// roleOrModule = 'role' or 'module' ( or anything not 'role' )
self.setOrClear( r, roleOrModule, e.target.checked );
};self.setOrClear = function( r, roleOrModule, pushOrRemove ) {
var method = ( roleOrModule === 'role' ) ? 'chosenRoles' : 'chosenModule';
var pushRemove = ( pushOrRemove ) ? 'push' : 'remove';
_.each( self.userDatabases(), function (d) {
if ( d[ method ].indexOf( r ) === -1) {
d[ method ][ pushRemove ](r);
}
});
};ko.computed(function () {
var eachMethod = ['userRoles','userModules'];
var chosenMethod = ['chosenRoles','chosenModules'];
var allMethod = ['allRolesSelected', 'allModulesSelected'];
for (var i=0, l = eachMethod.length; i < l; i++) {
_.each(self[ eachMethod[i] ], function (r) {
var all = self.userDatabases().length > 0 && _.every(self.userDatabases(), function (d) {
return d[ chosenMethod[i] ].indexOf(r) > -1;
});
if (all) {
if (self[ allMethod[i] ].indexOf(r) === -1) {
self[ allMethod[i] ].push(r);
}
} else {
self[ allMethod[i] ].remove(r);
}
});
}
});Context
StackExchange Code Review Q#77708, answer score: 2
Revisions (0)
No revisions yet.