Skip to content

Commit e461be6

Browse files
fix: refactor sortValues to resolve biome lint errors
- Extract _svGetCache, _svSetCache, _sortValuesColdPath, _svCmpAsc, _svCmpDesc private helpers to reduce cognitive complexity of sortValues from 26 to 1 - Replace nested ternary comparators with explicit if-return statements - Collapse else-if pattern in _svSetCache as required by useCollapsedElseIf Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 5604e0e commit e461be6

1 file changed

Lines changed: 49 additions & 56 deletions

File tree

src/core/series.ts

Lines changed: 49 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -724,31 +724,47 @@ export class Series<T extends Scalar = Scalar> {
724724

725725
// ─── sorting ─────────────────────────────────────────────────────────────
726726

727-
/** Return a new Series sorted by values. */
728-
sortValues(ascending = true, naPosition: "first" | "last" = "last"): Series<T> {
729-
// ── Per-instance cache: named properties for direct access on the hot path ──
730-
// Eliminates the O(n) partition + gather + Object.freeze spreads on all repeat
731-
// calls with the same parameters. AL=ascending+last, AF=ascending+first,
732-
// DL=descending+last, DF=descending+first.
727+
/** Retrieve a cached sortValues result, or null if not yet computed. */
728+
private _svGetCache(ascending: boolean, naPosition: "first" | "last"): Series<T> | null {
733729
if (ascending) {
734-
const hit = naPosition === "last" ? this._svCacheAL : this._svCacheAF;
735-
if (hit !== null) {
736-
return hit;
730+
return naPosition === "last" ? this._svCacheAL : this._svCacheAF;
731+
}
732+
return naPosition === "last" ? this._svCacheDL : this._svCacheDF;
733+
}
734+
735+
/** Store a sortValues result in the appropriate named cache slot. */
736+
private _svSetCache(ascending: boolean, naPosition: "first" | "last", result: Series<T>): void {
737+
if (ascending) {
738+
if (naPosition === "last") {
739+
this._svCacheAL = result;
740+
} else {
741+
this._svCacheAF = result;
737742
}
743+
} else if (naPosition === "last") {
744+
this._svCacheDL = result;
738745
} else {
739-
const hit = naPosition === "last" ? this._svCacheDL : this._svCacheDF;
740-
if (hit !== null) {
741-
return hit;
742-
}
746+
this._svCacheDF = result;
743747
}
748+
}
744749

745-
// ── Cold path: comparison sort (runs once per unique ascending+naPosition) ──
746-
// Using Array.prototype.sort keeps the function body compact so the JIT can
747-
// inline and specialise the per-instance cache-hit path above.
748-
const n = this._values.length;
749-
const vals = this._values;
750+
/** Ascending comparator for sortable values. */
751+
private static _svCmpAsc(a: number | string | boolean, b: number | string | boolean): number {
752+
if (a < b) return -1;
753+
if (a > b) return 1;
754+
return 0;
755+
}
750756

751-
// Partition: separate NaN/null/undefined from finite/sortable values.
757+
/** Descending comparator for sortable values. */
758+
private static _svCmpDesc(a: number | string | boolean, b: number | string | boolean): number {
759+
if (a > b) return -1;
760+
if (a < b) return 1;
761+
return 0;
762+
}
763+
764+
/** Cold path: partition, sort, and build the sorted Series. */
765+
private _sortValuesColdPath(ascending: boolean, naPosition: "first" | "last"): Series<T> {
766+
const vals = this._values;
767+
const n = vals.length;
752768
const finIdx: number[] = [];
753769
const nanIdx: number[] = [];
754770
for (let i = 0; i < n; i++) {
@@ -759,48 +775,25 @@ export class Series<T extends Scalar = Scalar> {
759775
finIdx.push(i);
760776
}
761777
}
762-
763-
// Sort finite indices by their corresponding values.
764-
if (ascending) {
765-
finIdx.sort((a, b) => {
766-
const av = vals[a] as number | string | boolean;
767-
const bv = vals[b] as number | string | boolean;
768-
return av < bv ? -1 : av > bv ? 1 : 0;
769-
});
770-
} else {
771-
finIdx.sort((a, b) => {
772-
const av = vals[a] as number | string | boolean;
773-
const bv = vals[b] as number | string | boolean;
774-
return av > bv ? -1 : av < bv ? 1 : 0;
775-
});
776-
}
777-
778-
// Build output permutation: NaN slots go first or last per naPosition.
778+
const cmp = ascending ? Series._svCmpAsc : Series._svCmpDesc;
779+
finIdx.sort((a, b) => cmp(vals[a] as number | string | boolean, vals[b] as number | string | boolean));
779780
const perm = naPosition === "first" ? [...nanIdx, ...finIdx] : [...finIdx, ...nanIdx];
780-
const outData = perm.map((i) => vals[i] as T);
781-
const outIndex = this.index.take(perm);
782-
783-
const result = new Series<T>({
784-
data: outData,
785-
index: outIndex,
781+
return new Series<T>({
782+
data: perm.map((i) => vals[i] as T),
783+
index: this.index.take(perm),
786784
dtype: this.dtype,
787785
name: this.name,
788786
});
787+
}
789788

790-
// Save to per-instance cache so repeat calls are O(1).
791-
if (ascending) {
792-
if (naPosition === "last") {
793-
this._svCacheAL = result;
794-
} else {
795-
this._svCacheAF = result;
796-
}
797-
} else {
798-
if (naPosition === "last") {
799-
this._svCacheDL = result;
800-
} else {
801-
this._svCacheDF = result;
802-
}
803-
}
789+
/** Return a new Series sorted by values. */
790+
sortValues(ascending = true, naPosition: "first" | "last" = "last"): Series<T> {
791+
// ── Per-instance cache: named properties for direct access on the hot path ──
792+
// AL=ascending+last, AF=ascending+first, DL=descending+last, DF=descending+first.
793+
const hit = this._svGetCache(ascending, naPosition);
794+
if (hit !== null) return hit;
795+
const result = this._sortValuesColdPath(ascending, naPosition);
796+
this._svSetCache(ascending, naPosition, result);
804797
return result;
805798
}
806799

0 commit comments

Comments
 (0)