4
\$\begingroup\$

I'm writing a Matrix class, and I want to be able to construct a Matrix in two ways:

  1. Supply a 2D array that the Matrix can use internally
  2. Supply the dimensions of the Matrix, and the value to fill each cell with.

In most languages, the obvious solution would be to overload the constructor, having 1 version of it that takes an array, and another that takes the dimensions and fill value.

Unfortunately, since JavaScript, which Typescript compiles into, doesn't support function overloading, neither does Typescript. The best I could do was declare the signatures for each of the constructor versions, then manually deal with the values internally. This is the constructor that I ended up with:

export class Matrix<T> {
        private dimensions: DimensionsType;
        private rows: T[][];

        constructor(arr: T[][]);
        constructor(width: number, height: number, defaultFill: T);

        constructor(arrOrWidth: T[][] | number, height?: number, defaultFill?: T) {
            if (arrOrWidth instanceof Array) {
                if (Matrix.verifyRowLengths(arrOrWidth)) {
                    this.rows = arrOrWidth;

                } else {
                    throw new Error("All rows must be the same length!");
                }

            } else {
                if (height !== undefined && defaultFill !== undefined) {
                    this.rows = Matrix.createMatrixArray(height, arrOrWidth, defaultFill);

                } else {
                    throw new Error(
                        "If you don't pass an array, you must specify the dimensions and fill value.");
                }
            }

            this.dimensions = Matrix.calcArrayDimensions(this.rows);
        }
        ...

Is this the safest way to achieve this? I'm mainly looking for suggestions on setting up the constructor, but any critique will be appreciated.

For context, and to make the code complete, here's the full module. Note, I'm in the middle of switching from using a 1D array to simulate a 2D array, and have a couple methods commented out, so it's a tad messy:

module Matrix {
    type DimensionsType = { width: number; height: number };

    export class Matrix<T> {
        private dimensions: DimensionsType;
        private rows: T[][];

        constructor(arr: T[][]);
        constructor(width: number, height: number, defaultFill: T);

        constructor(arrOrWidth: T[][] | number, height?: number, defaultFill?: T) {
            if (arrOrWidth instanceof Array) {
                if (Matrix.verifyRowLengths(arrOrWidth)) {
                    this.rows = arrOrWidth;

                } else {
                    throw new Error("All rows must be the same length!");
                }

            } else {
                if (height !== undefined && defaultFill !== undefined) {
                    this.rows = Matrix.createMatrixArray(height, arrOrWidth, defaultFill);

                } else {
                    throw new Error(
                        "If you don't pass an array, you must specify the dimensions and fill value.");
                }
            }

            this.dimensions = Matrix.calcArrayDimensions(this.rows);
        }

        public getWidth(): number {
            return this.dimensions.width;
        }

        public getHeight(): number {
            return this.dimensions.height;
        }

        public setCell(row: number, col: number, data: T) {
            this.assertIsInBounds(row, col);

            //this.rows[this.getCellIndex(row, col)] = data;
            this.rows[row][col] = data;
        }

        public getCell(row: number, col: number): T {
            this.assertIsInBounds(row, col);

            //return this.rows[this.getCellIndex(row, col)];
            return this.rows[row][col];
        }

        /*public plus(otherMatrix: Matrix<T>) {
            if (otherMatrix.getWidth() !== this.getWidth() || otherMatrix.getHeight() !== this.getHeight()) throw new Error(
                "Matrices must be the same size to add!");

        }*/

        public map<U>(f: (T) => U): Matrix<U> {
            let newRows: U[][] = [];

            for (let row of this.rows) {
                let newRow: U[] = [];

                for (let col of row) {
                    let newData = f(col);

                    newRow.push(newData);
                }

                newRows.push(newRow);
            }

            return new Matrix(newRows);
        }

        public toString(): string {
            return Matrix.format2D(this.rows);
        }

        private cellIsInBounds(row: number, col: number): boolean {
            return row >= 0 && row < this.dimensions.height
                && col >= 0 && col < this.dimensions.width;
        }

        private getCellIndex(row: number, col: number): number {
            return row * this.dimensions.width + col;
        }

        private outOfBoundsError(row: number, col: number): Error {
            return new Error(
                "Cell out of bounds: (" + row + ", " + col + ")"
            );
        }

        private assertIsInBounds(row, col) {
            if (!this.cellIsInBounds(row, col)) {
                throw this.outOfBoundsError(row, col);
            }
        }

        //Return null on bad dimensions
        /**
         * Does not check for correctness of row lengths.
         * Assumes them to be correct
         */
        private static calcArrayDimensions<T>(arr: T[][]): DimensionsType {
            if (arr.length === 0) {
                return { width: 0, height: 0 };
            }

            return {
                width:  arr[0].length,
                height: arr.length
            };
        }

        private static verifyRowLengths<T>(arr: T[][]): boolean {
            if (arr.length === 0) {
                return true;
            }

            let targetLength = arr[0].length;
            for (let rowN = 1; rowN < arr.length; rowN++) {
                let row = arr[rowN];

                if (row.length !== targetLength) {
                    return false;
                }
            }

            return true;
        }

        private static createMatrixArray<T>(nRows: number, nCols: number, fill: T): T[][] {
            let rows = [];
            for (let nRow = 0; nRow < nRows; nRow++) {
                let row = [];

                for (let nCol = 0; nCol < nCols; nCol++) {
                    row.push(fill);
                }

                rows.push(row);
            }

            return rows;
        }

        //private static mapBoth

        //Stolen from http://actionsnippet.com/?p=1227
        private static format2D<T>(a: T[][]): string {
            var str: string = "[";
            for (var i = 0; i < a.length; i++) {
                str += "[" + a[i].toString() + "],";
            }
            str = str.substr(0, str.length - 1);
            str += "]";
            return str;
        }
    }


}
\$\endgroup\$
7
\$\begingroup\$

A few of thoughts:

  • You don't need to declare types for the arguments in the implementation of your constructor, just the overloads (which precede the implementation).

  • I wouldn't use a separate object for the height and width, why not declare them as private members?

  • Consider throwing TypeError instead of plain Error if your constructor can't figure the arguments out.

  • Consider replacing private static verifyRowLengths() with a more general health-test method named checkInvariants(). There are more tests it can do for you, e.g. are the types of all elements the same?

  • Optimize ordering of the overload detection logic to avoid duplicate code. In doing this you can mutate the arguments and their types.

Here's a rough outline

export class Matrix<T> {
    private width: number;
    private height: number;
    private rows: T[][];

    constructor(arr: T[][]);
    constructor(width: number, height: number, defaultFill: T);
    constructor(a1, a2?, a3?) {
        if (typeof a1 === 'number' && typeof a2 == 'number') {
            a1 = Matrix.createRows( a1, a2 )
        }
        if (a1 instanceof Array) {
            this.rows = a1;
            //...
        } else {
            throw new TypeError("Unxpected arguments to Matrix constructor")
        }
        this.checkInvariants()
    }

    private static createRows<T>(width: number, height: number) : T[][] {
        return null;
    }

    private checkInvariants() : void {
        // ...
    }
}

Finally...

  • Consider public static methods which return a new Matrix instead of constructors overloads.

e.g.

export class Matrix<T> {
    ...
    public static diagonal<T>( size: number ) Matrix<T> { // ...
           }
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.