Am I using the Visitor Pattern correctly?

Question

I'm not sure if SO is the right place to post software design questions, so if there is a better place please let me know and I'll move this post there.

TL;DR: I have a class that is going to have a lot of methods. I don't want it to become bloated, so I want to find away to extract these methods using the Visitor pattern. However, I'm not sure if this is the intent of the pattern, or if I am mis-using it.

I'm playing around with a Matrix class right now. I keep wanting to add new methods to it, but I don't want to wind up with a bloated large class.

Some of these methods might be:

• getInverse(): Matrix
• getRREF(): Matrix
• transpose(): Matrix
• getDeterminant(): Matrix
• subtract(m: Matrix): Matrix
• multiply(m: Matrix): Matrix
• The list can go on and on and on.

So instead, what I've been thinking about doing is using the Visitor Pattern that I just learned about, but I'm worried that I'm using it incorrectly, or over-using it. Basically my plan is to expose a minimum interface for Matrix like this:

type VectorForEachCallback = (val: number, idx1: number) => void;

type VectorMapCallback = (val: number, idx1: number) => number;

type VectorReduceCallback<T> = (
  prevVal: T,
  currVal: number,
  idx1: number,
  vec: IVector
) => T;

interface IVector {
  size(): number;

  getValue(idx1: number): number;

  setValue(idx1: number, val: number): number;

  forEach(cb: VectorForEachCallback): void;

  map(cb: VectorMapCallback): void;

  dot(other: IVector): number;

  reduce<T = number>(cb: VectorReduceCallback<T>, initialValue: T): T;

  toArray(): number[];
}


interface IMatrix {
  isSquare(): boolean;

  getValue(row1: number, col1: number): number;

  getNumRows(): number;

  getNumCols(): number;

  getRows(): IVector[];

  getCols(): IVector[];

  getRow(idx1: number): IVector;

  getCol(idx1: number): IVector;

  forEach(cb: (val: number, row: number, col: number) => void): void;

  map(cb: (val: number, row: number, col: number) => number): IMatrix;

  accept<T>(visitor: IMatrixVisitor<T>): T
}

Then I would have a MatrixVisitor interface as follows:

interface IMatrixVisitor<T> {
  visit(m: IMatrix): T;
}

Some example visitors might be:

class MatrixMinorVisitor implements IMatrixVisitor<IMatrix> {
  row: number;
  col: number;

  constructor(row: number, col: number) {
    this.row = row;
    this.col = col;
  }

  visit(m: IMatrix): IMatrix {
    const result = ConcreteMatrix.makeZero(m.getNumRows() - 1, m.getNumCols() - 1);

    m.forEach((val, oldRow, oldCol) => {
      if (oldRow === this.row || oldCol === this.col) return;
      else {
        const newRow = oldRow < this.row ? oldRow : oldRow - 1;
        const newCol = oldCol < this.col ? oldCol : oldCol - 1;

        result.setValue(newRow, newCol, val);
      }
    });

    return result;
  }
}

class MatrixDeterminantVisitor implements IMatrixVisitor<number> {
  visit(m: IMatrix): number {
    if (!m.isSquare()) throw new Error("must be square");

    if (m.getNumRows() === 1) return m.getValue(1, 1);
    else {

      return m.getCols().reduce((sum, col, col0) => {
        const minorVisitor = new MatrixMinorVisitor(1, col0 + 1);
        const minor = minorVisitor.visit(m);
        const minorDet = this.visit(minor)
        const factor = (col0 + 1) % 2 === 0 ? -col.getValue(1) : col.getValue(1);
        return sum + factor * minorDet;
      }, 0);
    }
  }
}

As a final example, I could calculate a matrix determinant as follows:

function main() {
  const matrix = new ConcreteMatrix([[1, 2], [3, 4]]);

  const det = matrix.accept(new MatrixDeterminantVisitor());
  console.log(`The determinant is ${det}!`);
}

Answer 1

You don't have a complex structure, in the sense that you have to deal only with the Matrix class and not with mixture of classes or with composite objects.

For this reason you don't need the double dispatch feature provided by the couple of methods visit and accept visitor.

I would get rid of the acceptVisitor method on the matrix interface and refactor the visitor interface to be an "IMatrixCommand" and rename the "visit" method to "run" or "evaluate"