The main distinction seems to be in the increment value returned, depending on whether the current id is present in the this.userShelf.courseIds
array.
countSelectNotEnrolled(selected: any) {
this.selectedNotEnrolled = selected.reduce((count, id) => {
return this.hasSteps(id)
? count + (this.userShelf.courseIds.indexOf(id) === -1 ? 1 : 0)
// ^^^^^
: count;
}, 0);
this.selectedEnrolled = selected.reduce((count, id) => {
return this.hasSteps(id)
? count + (this.userShelf.courseIds.indexOf(id) === -1 ? 0 : 1)
// ^^^^^
: count;
}, 0);
}
A more general approach to refactoring such patterns involves extracting the difference as a dynamic aspect during function creation.
You can create a higher-order function that returns another function, taking the increment value as an argument. This way, you can create two functions for summing counts, one for selectedNotEnrolled
and the other for selectedEnrolled
.
Note: Using Array.prototype.includes
is cleaner than Array.prototype.indexOf
when checking array membership.
function createEnrolmentSum( // Accepts dynamic setup logic as arguments
incrementIfSelected,
incrementIfUnselected = incrementIfSelected === 1 ? 0 : 1
) {
return function (selected) { // <--- Returns a function with common logic
return selected.reduce((count, id) => {
return this.hasSteps(id)
? count +
(this.userShelf.courseIds.includes(id) // <-- Cleaner includes method
? incrementIfSelected
: incrementIfUnselected)
: count;
}, 0);
};
}
// ...
// Instance methods for proper `this` binding
getEnrolledSum = createEnrolmentSum(1);
getNotEnrolledSum = createEnrolmentSum(0);
countSelectNotEnrolled(selected: any) {
this.selectedNotEnrolled = this.getNotEnrolledSum();
this.selectedEnrolled = this.getEnrolledSum();
}
This demonstration shows how similar code could be refactored. The code is not very readable due to using bare numbers like `1` or `0`:
// Lack of readability due to unclear integer values
getEnrolledSum = createEnrolmentSum(1);
getNotEnrolledSum = createEnrolmentSum(0);
To improve clarity, you may use a configuration object:
getEnrolledSum = createEnrolmentSum({
incrementIfSelected: 1,
incrementIfUnselected: 0
});
getNotEnrolledSum = createEnrolmentSum({
incrementIfSelected: 0,
incrementIfUnselected: 1
});
However, the complexity remains high despite being DRY. For your situation, computing both sums in a single loop might be a better starting solution for improved performance and simplicity:
countSelectNotEnrolled(selected) {
let selectedNotEnrolled = 0,
selectedEnrolled = 0;
for (const id of selected) {
if (this.hasSteps(id)) {
if (this.userShelf.courseIds.includes(id)) {
selectedEnrolled += 1;
} else {
selectedNotEnrolled += 1;
}
}
}
this.selectedNotEnrolled = selectedNotEnrolled;
this.selectedEnrolled = selectedEnrolled;
}
If still preferring array reduction, an object to carry variables through iterations can reduce nesting:
countSelectNotEnrolled(selected) {
const { selectedNotEnrolled, selectedEnrolled } = selected.reduce(
(result, id) => {
if (this.hasSteps(id)) {
if (this.userShelf.courseIds.includes(id)) {
result.selectedEnrolled += 1;
} else {
result.selectedNotEnrolled += 1;
}
}
return result;
},
{ selectedNotEnrolled: 0, selectedEnrolled: 0 }
);
this.selectedNotEnrolled = selectedNotEnrolled;
this.selectedEnrolled = selectedEnrolled;
}
For increased readability, filter out ids without steps first:
countSelectNotEnrolled(selected) {
const { selectedNotEnrolled, selectedEnrolled } = selected.filter(this.hasSteps).reduce(
(result, id) => {
if (this.userShelf.courseIds.includes(id)) {
result.selectedEnrolled += 1;
} else {
result.selectedNotEnrolled += 1;
}
return result;
},
{ selectedNotEnrolled: 0, selectedEnrolled: 0 }
);
this.selectedNotEnrolled = selectedNotEnrolled;
this.selectedEnrolled = selectedEnrolled;
}
Utilizing length of filtered arrays for enrolled and not-enrolled ids provides a concise alternative:
countSelectNotEnrolled(selected) {
const selectedWithSteps = selected.filter(this.hasSteps);
this.selectedNotEnrolled = selectedWithSteps.filter(
id => !this.userShelf.courseIds.includes(id)
).length;
this.selectedEnrolled = selectedWithSteps.length - this.selectedNotEnrolled;
}
Alternatively, expressing enrolled and not-enrolled counts intertwined offers a simplified approach:
countSelectNotEnrolled(selected) {
const selectedWithSteps = selected.filter(this.hasSteps);
this.selectedNotEnrolled = selectedWithSteps.filter(
id => !this.userShelf.courseIds.includes(id)
).length;
this.selectedEnrolled = selectedWithSteps.length - this.selectedNotEnrolled;
}
In conclusion, prioritize code readability over specific patterns. JavaScript is meant for humans to understand easily, minimizing cognitive load🙂