Skip to content

Commit

Permalink
Merge pull request #79 from molgenis/fix/sortNullValues
Browse files Browse the repository at this point in the history
fix:sort null values
  • Loading branch information
bartcharbon authored Jun 2, 2022
2 parents 43a2775 + 5f082f0 commit 5bfe5d2
Show file tree
Hide file tree
Showing 5 changed files with 192 additions and 73 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@molgenis/vip-report-api",
"version": "3.3.0",
"version": "3.3.1",
"description": "TypeScript Report API for Variant Call Format (VCF) Report Templates",
"scripts": {
"build": "tsc --build",
Expand Down
66 changes: 1 addition & 65 deletions src/ApiClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
} from "./Api";
import { Metadata as RecordMetadata, Record } from "@molgenis/vip-report-vcf/src/Vcf";
import { FieldMetadata, NestedFieldMetadata } from "@molgenis/vip-report-vcf/src/MetadataParser";
import { compareAsc, compareDesc } from "./compare";

export interface ReportData {
metadata: Metadata;
Expand Down Expand Up @@ -211,64 +212,6 @@ export class ApiClient implements Api {
}
}

function compareAsc(a: unknown, b: unknown): number {
if (a === null) {
return b === null ? 0 : -1;
} else if (b === null) {
return 1;
} else if (typeof a === "number" && typeof b === "number") {
return compareAscNumber(a, b);
} else if (typeof a === "string" && typeof b === "string") {
return compareAscString(a, b);
} else if (typeof a === "boolean" && typeof b === "boolean") {
return compareAscBoolean(a, b);
} else if (Array.isArray(a) && Array.isArray(b)) {
return compareAscArray(a, b);
} else {
throw new CompareTypeError(a);
}
}

function compareAscNumber(a: number, b: number): number {
return a - b;
}

function compareAscString(a: string, b: string): number {
return a.toUpperCase().localeCompare(b.toUpperCase());
}

function compareAscBoolean(a: boolean, b: boolean): number {
if (a === b) {
return 0;
} else {
return a ? -1 : 1;
}
}

function compareAscArray(a: unknown[], b: unknown[]): number {
if (a.length === 0) {
return b.length === 0 ? 0 : -1;
} else if (b.length === 0) {
return 1;
} else {
const firstA = a.find((value) => value !== null);
if (firstA !== undefined && typeof firstA !== "number" && typeof firstA !== "string")
throw new CompareTypeError(firstA);
const firstB = b.find((value) => value !== null);
if (firstB !== undefined && typeof firstB !== "number" && typeof firstB !== "string")
throw new CompareTypeError(firstB);

// sort on first array item
const sortedA = a.slice().sort();
const sortedB = b.slice().sort();
return compareAsc(sortedA[0], sortedB[0]);
}
}

function compareDesc(a: unknown, b: unknown) {
return compareAsc(b, a);
}

function getCompareFn(sortOrder: SortOrder): CompareFn {
let compareFn;
if (sortOrder.compare === "asc" || sortOrder.compare === null || sortOrder.compare === undefined) {
Expand All @@ -288,13 +231,6 @@ function getCompareFn(sortOrder: SortOrder): CompareFn {
return compareFn;
}

class CompareTypeError extends Error {
constructor(value: unknown) {
super(`can't compare values of type '${typeof value}'. consider providing a custom compare function.`);
this.name = "CompareTypeError";
}
}

class UnknownFieldError extends Error {
constructor(fieldId: string, path: Path) {
super(`unknown field '${fieldId}' in path '[${path.join(",")}]'`);
Expand Down
24 changes: 17 additions & 7 deletions src/__tests__/ApiClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ test("get - all records sorted on n.n_bool3", async () => {
},
};
const records = await api.getRecords(params);
expect(records).toEqual({ ...sortAllExpected, ...{ items: [record0, record1] } });
expect(records).toEqual({ ...sortAllExpected, ...{ items: [record1, record0] } });
});

test("get - all records sorted on n.n_bool4", async () => {
Expand Down Expand Up @@ -561,7 +561,7 @@ test("get - all records sorted on n.n_bool6", async () => {
},
};
const records = await api.getRecords(params);
expect(records).toEqual({ ...sortAllExpected, ...{ items: [record0, record1] } });
expect(records).toEqual({ ...sortAllExpected, ...{ items: [record1, record0] } });
});

test("get - all records sorted on n.n_bool7", async () => {
Expand All @@ -579,7 +579,7 @@ test("get - all records sorted on n.n_bool7", async () => {
},
};
const records = await api.getRecords(params);
expect(records).toEqual({ ...sortAllExpected, ...{ items: [record0, record1] } });
expect(records).toEqual({ ...sortAllExpected, ...{ items: [record1, record0] } });
});

test("get - all records sorted on n.n_bool8", async () => {
Expand Down Expand Up @@ -726,16 +726,28 @@ test("get - all records sorted on n.n_array0", async () => {
expect(records).toEqual({ ...sortAllExpected, ...{ items: [record1, record0] } });
});

test("get - all records sorted on n.n_object0.n_string2", async () => {
test("get - all records sorted on n.n_object0.n_string2 ascending", async () => {
const params: Params = {
sort: {
property: nString2Meta,
compare: "asc",
},
};
const records = await api.getRecords(params);
expect(records).toEqual({ ...sortAllExpected, ...{ items: [record1, record0] } });
});

test("get - all records sorted on n.n_object0.n_string2 descending", async () => {
const params: Params = {
sort: {
property: nString2Meta,
compare: "desc",
},
};
const records = await api.getRecords(params);
expect(records).toEqual({ ...sortAllExpected, ...{ items: [record0, record1] } });
});

test("get - all records sorted on n.n_object0 throws an error", async () => {
const params: Params = {
sort: {
Expand All @@ -749,9 +761,7 @@ test("get - all records sorted on n.n_object0 throws an error", async () => {
},
},
};
await expect(api.getRecords(params)).rejects.toThrow(
"can't compare values of type 'object'. consider providing a custom compare function."
);
await expect(api.getRecords(params)).rejects.toThrow("can't compare values of type 'object'.");
});

test("get - all records sorted ascending on position implicitly", async () => {
Expand Down
77 changes: 77 additions & 0 deletions src/__tests__/compare.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import { describe, expect, test } from "vitest";
import { compareAsc, compareDesc } from "../compare";

describe("compare functions", () => {
test("sort strings ascending", () => {
expect(["b", null, "A", "c", "b"].sort(compareAsc)).toStrictEqual(["A", "b", "b", "c", null]);
});

test("sort strings descending", () => {
expect(["b", null, null, "A", "c"].sort(compareDesc)).toStrictEqual(["c", "b", "A", null, null]);
});

test("sort numbers ascending", () => {
expect([2, null, 1, 3].sort(compareAsc)).toStrictEqual([1, 2, 3, null]);
});

test("sort numbers descending", () => {
expect([null, 2, 1, 1, 3].sort(compareDesc)).toStrictEqual([3, 2, 1, 1, null]);
});

test("sort boolean ascending", () => {
expect([true, null, true, false].sort(compareAsc)).toStrictEqual([true, true, false, null]);
});

test("sort boolean descending", () => {
expect([true, null, false].sort(compareDesc)).toStrictEqual([false, true, null]);
});

test("sort array ascending", () => {
const array0 = [3, 2, 4];
const array1 = [2, null, 1, 3];
const array2: string[] = [];
const array3: string[] = [];
const array4 = [null, 4, 5, 6];
const array5 = [null];
expect([array0, array1, array2, array3, array4, array5].sort(compareAsc)).toStrictEqual([
array1,
array0,
array4,
array5,
array2,
array3,
]);
});

test("sort array descending", () => {
const array0 = [3, 2, 4];
const array1 = [2, null, 1, 3];
const array2: string[] = [];
const array3: string[] = [];
const array4 = [null, 4, 5, 6];
const array5 = [null];
expect([array0, array1, array2, array3, array4, array5].sort(compareDesc)).toStrictEqual([
array4,
array0,
array1,
array5,
array2,
array3,
]);
});

test("sort arrays throws an error", () => {
expect(() => [[{ x: 0, y: 1 }], [{ x: 1, y: 2 }]].sort(compareAsc)).toThrowError(
"can't compare values of type 'object'."
);
});

test("sort object throws an error", () => {
expect(() =>
[
{ x: 0, y: 1 },
{ x: 1, y: 2 },
].sort(compareAsc)
).toThrowError("can't compare values of type 'object'.");
});
});
96 changes: 96 additions & 0 deletions src/compare.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
class CompareTypeError extends Error {
constructor(value: unknown) {
super(`can't compare values of type '${typeof value}'.`);
this.name = "CompareTypeError";
}
}

export function compareDesc(a: unknown, b: unknown) {
if (a === null) {
return b === null ? 0 : 1;
} else if (b === null) {
return -1;
} else if (Array.isArray(a) && Array.isArray(b)) {
return compareDescArray(a, b);
} else {
return compareAscNonNullPrimitives(b, a);
}
}

export function compareAsc(a: unknown, b: unknown): number {
if (a === null) {
return b === null ? 0 : 1;
} else if (b === null) {
return -1;
} else if (Array.isArray(a) && Array.isArray(b)) {
return compareAscArray(a, b);
} else {
return compareAscNonNullPrimitives(a, b);
}
}

function compareAscNonNullPrimitives(a: unknown, b: unknown): number {
if (typeof a === "number" && typeof b === "number") {
return compareAscNumber(a, b);
} else if (typeof a === "string" && typeof b === "string") {
return compareAscString(a, b);
} else if (typeof a === "boolean" && typeof b === "boolean") {
return compareAscBoolean(a, b);
} else {
throw new CompareTypeError(a);
}
}

function compareAscNumber(a: number, b: number): number {
return a - b;
}

function compareAscString(a: string, b: string): number {
return a.toUpperCase().localeCompare(b.toUpperCase());
}

function compareAscBoolean(a: boolean, b: boolean): number {
if (a === b) {
return 0;
} else {
return a ? -1 : 1;
}
}

function checkArrays(arrays: unknown[][]) {
for (const array of arrays) {
const firstA = array.find((value) => value !== null);
if (firstA !== undefined && typeof firstA !== "number" && typeof firstA !== "string")
throw new CompareTypeError(firstA);
}
}

function compareAscArray(a: unknown[], b: unknown[]): number {
if (a.length === 0) {
return b.length === 0 ? 0 : 1;
} else if (b.length === 0) {
return -1;
} else {
checkArrays([a, b]);

// sort on first array item
const sortedA = a.slice().sort(compareAsc);
const sortedB = b.slice().sort(compareAsc);
return compareAsc(sortedA[0], sortedB[0]);
}
}

function compareDescArray(a: unknown[], b: unknown[]): number {
if (a.length === 0) {
return b.length === 0 ? 0 : 1;
} else if (b.length === 0) {
return -1;
} else {
checkArrays([a, b]);

// sort on first array item
const sortedA = a.slice().sort(compareDesc);
const sortedB = b.slice().sort(compareDesc);
return compareDesc(sortedA[0], sortedB[0]);
}
}

0 comments on commit 5bfe5d2

Please sign in to comment.