Provided is the typescript function, where we need to reduce the cyclometric complexity. I am thinking off to provide the inverted if but that doesn't help much.
updateSort(s: Sort) {
if (s.active !== this.personSort.active || s.direction !== this.personSort.direction) {
let sortFunc: (a: PersonInfo, b: PersonInfo) => number;
switch (s.active) {
default:
case 'personName':
sortFunc = (a, b) => (a.name).localeCompare(b.name);
break;
case 'personAddress':
sortFunc = (a, b) => (a.pageTypeDescription.toLocaleString()).localeCompare(b.pageTypeDescription.toLocaleString());
break;
case 'personVisitors':
sortFunc = (a, b) => a.viewsCount - b.viewsCount;
break;
case 'personContacts':
sortFunc = (a, b) => a.clicksCount - b.clicksCount;
break;
}
if (s.direction === 'desc') {
this.sliceList.sort((a, b) => sortFunc(b, a));
} else {
this.sliceList.sort(sortFunc);
}
}
}
CodePudding user response:
As noted in the comments, you can invert the boolean logic to return early, which is always nicer to read and you can easily extract the switch statement into it's own function. E.g.
updateSort(s: Sort) {
if (s.active === this.personSort.active
&& s.direction === this.personSort.direction) {
return
}
const sortFunc = this.getSortFunc(s.active)
if (s.direction === 'desc') {
this.sliceList.sort((a, b) => sortFunc(b, a))
} else {
this.sliceList.sort(sortFunc)
}
}
getSortFunc(type: string): (a: PersonInfo, b: PersonInfo) => number {
// no need to repeat that as well
const localeCompare = (a: string, b: string) => a.localeCompare(b)
switch (type) {
default:
case 'personName':
return (a, b) => localeCompare(a.name, b.name)
case 'personAddress':
return (a, b) =>
localeCompare(
a.pageTypeDescription.toLocaleString(),
b.pageTypeDescription.toLocaleString()
)
case 'personVisitors':
return (a, b) => a.viewsCount - b.viewsCount
case 'personContacts':
return (a, b) => a.clicksCount - b.clicksCount
}
}
and an example of getting rid of the switch
entirely like noted by @caTS could be
localeCompare = (a: string, b: string) => a.localeCompare(b)
sortsFunctions: Record<string, (a: PersonInfo, b: PersonInfo) => number> = {
personName: (a, b) => this.localeCompare(a.name, b.name),
personAddress: (a, b) =>
this.localeCompare(
a.pageTypeDescription.toLocaleString(),
b.pageTypeDescription.toLocaleString()
),
personVisitors: (a, b) => a.viewsCount - b.viewsCount,
personContacts: (a, b) => a.clicksCount - b.clicksCount,
}
getSortFunc(type: string): (a: PersonInfo, b: PersonInfo) => number {
return this.sortsFunctions[type] ?? this.sortsFunctions.personName
}
There's more you could do, similar to 3limin4t0r you could also refactor into
// fully generic part
type Comparator<T> = (a: T, b: T) => number
const numCompare: Comparator<number> =
(a, b) => a - b
const localeCompare: Comparator<string> =
(a, b) => a.localeCompare(b)
const invert = <T>(comparator: Comparator<T>): Comparator<T> =>
(a, b) => comparator(b, a)
const compareProperties = <T, P>(propExtract: (object: T) => P, comparator: Comparator<P>): Comparator<T> =>
(a, b) => comparator(propExtract(a), propExtract(b))
// specific to the task
const sortFunctions: Record<string, Comparator<PersonInfo>> = {
personName: compareProperties(p => p.name, localeCompare),
personAddress: compareProperties(p => p.pageTypeDescription.toLocaleString(), localeCompare),
personVisitors: compareProperties(p => p.viewsCount, numCompare),
personContacts: compareProperties(p => p.clicksCount, numCompare),
}
const getSortFunc = (type: string) => sortFunctions[type] ?? sortFunctions.personName
const sortsEquals = (a: Sort, b: Sort) => a.active === b.active
&& a.direction == b.direction
class PersonPage {
personSort: Sort
sliceList: PersonInfo[]
updateSort(s: Sort) {
if (sortsEquals(s, this.personSort)) {
return
}
let sortFunc = getSortFunc(s.active)
if (s.direction === 'desc') {
sortFunc = invert(sortFunc)
}
this.sliceList.sort(sortFunc)
}
}
CodePudding user response:
I would personally extract a lot of the basic sorting stuff into their own general purpose helpers. This would reduce your method to:
updateSort(s: Sort) {
if (s.active === this.personSort.active && s.direction === this.personSort.direction) return;
const direction = s.direction === "desc" ? desc : asc;
this.sliceList.sort(direction((personInfo: PersonInfo) => {
switch (s.active) {
case "personAddress":
return personInfo.pageTypeDescription.toLocaleString();
case "personVisitors":
return personInfo.viewsCount;
case "personContacts":
return personInfo.clicksCount;
default:
return personInfo.name;
}
}));
}
This does require you to introduce the asc
and desc
helper functions. These functions take a function argument fn
and returns an ascending/descending comparator. When the comparator is invoked, fn
is applied to both a
and b
before they are passed to the comparator.
This allows us to remove a lot of the code "duplication".
// "duplicate" code
(a.pageTypeDescription.toLocaleString()).localeCompare(b.pageTypeDescription.toLocaleString())
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Here are the general purpose helper functions as I've defined them for this solution:
// Returns true if item has the given method, false otherwise.
function hasMethod(item: any, methodName: string | symbol): boolean {
// intentional `!=` usage, hits both `null` and `undefined`
return item != null && typeof item[methodName] === "function";
}
// Compares 2 items. Returns a negative number if A < B, returns a
// positive number if A > B, returns 0 otherwise. If both A and B
// have the method `localeCompare` then `a.localeCompare(b)` is
// used instead.
function compare(a: any, b: any): number {
const isLocaleComparable = hasMethod(a, "localeCompare") && hasMethod(b, "localeCompare");
if (isLocaleComparable) return a.localeCompare(b);
return -(a < b) || (a > b);
}
// Returns an ascending comparator that applies fn to both A and B
// before comparing the resulting values with each other.
function asc(fn: (item: any) => any) {
return function (a: any, b: any): number {
return compare(fn(a), fn(b));
};
}
// Returns an descending comparator that applies fn to both A and B
// before comparing the resulting values with each other.
function desc(fn: (item: any) => any) {
return function (a: any, b: any): number {
return compare(fn(b), fn(a));
};
}
Of course you're free to customize them further to fit your needs.