Go track - `matrix` problem

Exercism Support
1

Looking at the matrix_test.go - functions Test_New line 165 and BenchmarkNew line 285. In both cases we have compare matrix returned by New to nil. Is it even necessary? Shouldn’t checking the error should be enough. Problem is that it forces implementation of type Matrix - if I implement it as:

type Matrix struct {
	rNum int
	cNum int
	vals []int
}

Then I cannot return nil or something comparable to nil from New. All I can return is Matrix{} which cannot be compared to nil. It would be easier if New was returning pointer *Matrix. But that does not fly because test suite excepts it to be a Matrix.

Am I doing something wrong?
My first attempt was Matrix implemented as [][]int and in this case all is working:

	m0 := Matrix{}
	fmt.Println(m0 == nil)

is fine.

Can you fix the testcase - I really do not believe testcase should enforce implementation.

2

FWIW this exercise used *Matrix for years until four months ago when that was changed to Matrix.

3

Unfortunately, that commit breaks prior solutions :frowning_face: @andrerfcsantos may have more context here.

4

I can implement Matrix as a slice with row and col number being element 0 and 1. But as a matter of principle got == nil seems to me like a bad style. Especially if we do the error checking.

5

The reason the return type of New was changed from *Matrix to Matrix was to make the definition of Matrix as type Matrix [][]int slightly more viable and less weird to work with. In the mentoring of this exercise, a lot of students were doing type Matrix [][]int, which is a very natural way and simple way to solve the exercise, but you would then be faced with some unnecessary complications:

  1. Slices in Go are already pointers, so if you defined type Matrix [][]int and you have *Matrix, you are essentially getting a pointer to a pointer. This double indirection is cumbersome.
  2. Where the double indirection being weird was manifested was in the method declarations, where you are either forced to littler the code with dereferences such as (*m)[0], or you can do the dereferencing upfront such as mm := *m and just use mm. In both scenarios, the question started to become, “well, if I have a pointer that I always have to dereference, why have a pointer in the first place?”

It was decided then to make New just return a Matrix to allow for these simple solutions.

With that said, I’m ok with removing the nil checks to allow for more flexibility. @Tom-Niesytto feel free to create a PR with the change. The bot will close it, but I can reopen+merge it afterward. As you also correctly identified, the benchmarks should also be changed to not depend on the nil check. I think a good alternative to the nil check could be just checking if the lengths of rows and columns are correct for the benchmark, for the New test, I think just removing the test altogether is fine.