I really need bettter solution for this logic:
this.allClients.forEach(obj => {
if(obj.status === 2) {
this.numOfWaitingUsers.push(obj)
}
if(obj.status === 1) {
this.numOfInactiveUsers.push(obj)
}
if(obj.status === 0) {
this.numOfActiveUsers.push(obj)
}
if(obj.status === 3) {
this.numOfAClosedUsers.push(obj)
}
})
This is work perfect but i need better soluiton. i know to can be better with less code.
One solution would be to map from the status number to the array property name, like this:
const arrayNameByStatus = { // This could be an array, but I wasn't sure if
0: "numOfActiveUsers", // status codes were necessarily contiguous like
1: "numOfInactiveUsers", // they are in the question
2: "numOfWaitingUsers",
3: "numOfAClosedUsers",
};
for (const obj of this.allClients) {
const name = arrayNameByStatus[obj.status];
if (name) { // Remove this for an error if status is an unexpected value
this[name].push(obj);
}
}
Live Example:
const arrayNameByStatus = {
0: "numOfActiveUsers",
1: "numOfInactiveUsers",
2: "numOfWaitingUsers",
3: "numOfAClosedUsers",
};
class Example {
constructor() {
this.allClients = [
{status: 0},
{status: 2},
{status: 2},
];
this.numOfActiveUsers = [];
this.numOfInactiveUsers = [];
this.numOfWaitingUsers = [];
this.numOfAClosedUsers = [];
}
method() {
for (const obj of this.allClients) {
const name = arrayNameByStatus[obj.status];
if (name) {
this[name].push(obj);
}
}
}
}
const e = new Example();
e.method();
console.log(e);But, if you're going to index by status regularly, you might consider changing the structure of your object to support that directly. For instance, you might have a userCounts property that's an object with keys 0 through 3, which would let you index in directly:
// **IF** you change the structure so that `this` has a `userCounts`
// keyed by status:
for (const obj of this.allClients) {
const array = this.userCounts[obj.status];
if (array) { // Remove this for an error if status is an unexpected value
array.push(obj);
}
}
Live Example:
class Example {
constructor() {
this.allClients = [
{status: 0},
{status: 2},
{status: 2},
];
this.userCounts = {
0: [],
1: [],
2: [],
3: [],
};
}
method() {
for (const obj of this.allClients) {
const array = this.userCounts[obj.status];
if (array) {
array.push(obj);
}
}
}
}
const e = new Example();
e.method();
console.log(e);
array name byStatus, go for arrayByStatus.
this[propName]? Makes for tracing code and finding usage of variables/context members much more difficult, making refactoring more error prone.Honestly, the code you've posted looks perfectly fine. However, if you had lots more if statements to go through, you could optimize it like the following:
// use the corresponding status as the key to access each array
let dataContainer = {
0: this.numOfActiveUsers,
1: this.numOfInactiveUsers,
2: this.numOfWaitingUsers,
3: this.numOfAClosedUsers
}
this.allClients.forEach(obj => dataContainer[obj.status].push(obj))
dataContainer[obj.status] if it's possible to have a status value that doesn't map to any of these arrays.)Use a lookup table for your status codes:
// const countNames = {0:"numOfActiveUsers", 1:"numOfInactiveUsers", 2:"numOfWaitingUsers", 3:"numOfAClosedUsers"};
const countNames = ["numOfActiveUsers", "numOfInactiveUsers", "numOfWaitingUsers", "numOfAClosedUsers"];
for (const obj of this.allClients) {
const count = countNames[obj.status];
if (count)
this[count].push(obj)
}
elses…