Creating a status object based on the command type and action

Question

I need to create an object based on some properties of another given object. In my example setRules, add properties to object status.

Considering that the cases to be handled will increase increase I would like to figure out how to enhance readability.

I would like to know:

• Could a switch statement increase readability (please scroll code below to see version 02 of the script)?
• Any better way to write this from readability point of view?

// version 01
var status = {};
function setRules(cmd) {
 if (cmd.type === 'category' && cmd.action === 'add') {
 status = {
 app: {
 isEdit: false,
 hasPriority: true
 },
 category: {
 lock: false
 }
 };
 } else if (cmd.type === 'category' && cmd.action === 'edit') {
 status = {
 app: {
 isEdit: true,
 hasPriority: true
 },
 category: {
 lock: true
 }
 };
 } else if (cmd.type === 'category' && cmd.action === 'delete') {
 status = {
 app: {
 isEdit: true,
 hasPriority: false
 },
 category: {
 lock: false
 }
 };
 } else if (cmd.type === 'category' && cmd.action === 'read') {
 status = {
 app: {
 isEdit: false,
 hasPriority: false
 },
 category: {
 lock: false
 }
 };
 } else if (cmd.type === 'users' && cmd.action === 'read') {
 status = {
 app: {
 isEdit: false,
 hasPriority: false
 },
 category: {
 lock: false
 }
 };
 } else if (cmd.type === 'users' && cmd.action === 'edit') {
 status = {
 app: {
 isEdit: true,
 hasPriority: true
 },
 category: {
 lock: true
 }
 };
 }
}
setRules({type: 'users', action:'edit'});

// version 02
var status = {};
function setRules(cmd) {
 if(cmd.type === 'category'){
 switch (action) {
 case 'add':
 status = {
 app: {
 isEdit: false,
 hasPriority: true
 },
 category: {
 lock: false
 }
 };
 break;
 case 'edit':
 status = {
 app: {
 isEdit: true,
 hasPriority: true
 },
 category: {
 lock: true
 }
 };
 break;
 case 'delete':
 status = {
 app: {
 isEdit: true,
 hasPriority: false
 },
 category: {
 lock: false
 }
 };
 break;
 case 'read':
 status = {
 app: {
 isEdit: false,
 hasPriority: false
 },
 category: {
 lock: false
 }
 };
 break;
 }
 } else if (cmd.type === 'users') {
 switch(action){
 case 'read':
 status = {
 app: {
 isEdit: false,
 hasPriority: false
 },
 category: {
 lock: false
 }
 };
 break;
 case 'edit':
 status = {
 app: {
 isEdit: true,
 hasPriority: true
 },
 category: {
 lock: true
 }
 };
 break;
 }
 }
}
setRules({type: 'users', action:'edit'});

• SuperBiasedMan: Have you considered building status up front and then just setting the boolean values based on your cmd.type and action? I suspect you could make a much shorter and more readable code that way.
• njzk2: for one, it looks like the cmd.type is not really important.

Answer 1

If you know about the format of cmd you can merge the properties you're checking and check against both in one step.

status = and the structure of the Object it's being set to is repeated code, factor it out. When figuring out what to factor out you can almost imagine you're a dumb diff algorithm: what are the minimal text changes necessary to change between these forms? The answer is all the code that should be in that place, plus code to plug it into the thing that implements the commonalities of the form.

Don't be afraid to put code on one line when its complexity is low. Yes you'll need to refactor if the complexity grows, but worrying about possibilities without considering their likelihood is a path to insanity. Make the code look the best it can today.

A rewrite with those changes:

function setRules(cmd) {
 switch (cmd.type + '/' + cmd.action) {
 case 'category/add': setStatus(false, true, false); break;
 case 'category/edit': setStatus( true, true, true); break;
 case 'category/delete': setStatus( true, false, false); break;
 case 'category/read': setStatus(false, false, false); break;
 case 'users/read': setStatus(false, false, false); break;
 case 'users/edit': setStatus( true, true, true); break;
 }
}
function setStatus(edit, priority, lock) {
 status = {
 app: {isEdit: edit, hasPriority: priority},
 category: {lock: lock},
 };
}

• jelder: This is very concise, but it still requires the reader to frequently refer back to the definition setStatus().
• agweber: If the definition of setStatus is moved away from setRules I would just put a comment above the cases to display the arguments like a table header.
• njzk2: isLock or hasLock for consistency

Answer 2

One option is to use a decision table pattern:

var rules = {
 users: { 
 read : Status(),
 edit : Status({isEdit: true})
 }, 
 categories {
 read : Status({}),
 edit : Status({isEdit: true, hasPriority: true, lock: true}),
 delete : Status({isEdit: true}),
 add : Status({hasPriority: true})
 }
};
// sample usage
var status = rules[cmd.type][cmd.caction];
function Status(settings) {
 if(!this) return new Status(settings); // allow without new
 this.app = {};
 this.app.isEdit = settings.isEdit || false;
 this.app.hasPriority = settings.hasPriority || false
 this.category = settings.lock || false;
 Object.freeze(this); // make immutable
 Object.freeze(this.app);
}

By using immutable objects we also don't create a new copy for each element.

(削除) I can pretty up the code using ES2015 if you'd like. (削除ここまで)

Here is a modern version of the above code for Status minus allowing usage with new (per your request):

const Status = ({isEdit = false, hasPriority = false, category = false}) =>
 Object.freeze({ app: Object.freeze({isEdit, hasPriority }), category});

• Benjamin Gruenbaum: The decision table is acting as a factory by the way, only since objects are immutable all objects (6) are allocated only once and up front - so if in theory you make these choices very often it will likely save you some memory. You also don't have to worry about copying it or it being modified externally.
• Supr: Does that "allow without new" trick work in the browser? I imagine this would be window and you end up setting app and category on window before finally "returning" undefined?
• Benjamin Gruenbaum: @Supr good question! I assume strict mode in all code I write and make sure I'm in it. In strict mode this is undefined and not window. I can change that to a this instanceof Status instead perhaps
• Supr: Ahh, I see, that makes sense. I'm not too familiar with the specifics of strict mode so I did not realize. TIL.

Answer 3

As far as I can tell, there is a logic in your boolean results. If you always apply the same properties for the same action, I'd go with:

function getActionStatus(cmd){
 return {
 app: {
 isEdit: ['edit', 'add', 'delete'].indexOf(cmd.action) !== -1,
 hasPriority: ['edit', 'add'].indexOf(cmd.action) !== -1
 },
 category: {
 lock: ['edit'].indexOf(cmd.action) !== -1
 }
 }
}

If there will be some changes depending on type, and you can't figure out any pattern in your data, you can handle special cases this way:

function getStatus(cmd){
 var specialTypes = {
 user: function(status){
 status.lock = ['whatever', 'other'].indexOf(cmd.action) !== -1
 return status;
 }
 } 
 var status = getActionStatus(cmd);
 if(specialTypes[cmd.type]){
 status = specialTypes[cmd.type](status);
 }
 return status;
}