Skip to content

I think the Matrix problem would be better without the 'string' tests. #541

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
derifatives opened this issue May 10, 2017 · 4 comments
Closed

Comments

@derifatives
Copy link

As a beginner-intermediate Haskeller making my way through the exercises, this was the first time I had to submit an incomplete result and look up the answer. When I looked at it I didn't understand it, and I'm still not sure I understand it. [Basically, I think the type inference is coming from the other side of the shouldBe in the tests, is that right?] I never would have figured this out for myself.

I also don't feel it makes much sense in the context of the problem. The direct text of the problem is "Given a string representing a matrix of numbers, return the rows and columns of that matrix." Expecting that to work with a matrix of strings is not reasonable, particularly given that it's a much more difficult parsing problem than anything seen so far.

@petertseng
Copy link
Member

Even though the https://github.com/exercism/x-common/blob/master/exercises/matrix/canonical-data.json does not exist yet, what @derifatives says is true - the https://github.com/exercism/x-common/blob/master/exercises/matrix/description.md only says matrices of numbers, so the https://github.com/exercism/xhaskell/blob/master/exercises/matrix/test/Tests.hs#L76-L84 seem out-of-place.

If the maintainers of this track decide that the parsing questions are useful to have, they can be moved to a track-specific exercise, perhaps?

@petertseng
Copy link
Member

And my personal opinion:

Will have to wait until I've done this exercise.

@rbasso
Copy link
Contributor

rbasso commented May 10, 2017

@derifatives wrote:

Basically, I think the type inference is coming from the other side of the shouldBe in the tests, is that right?

I don't understand what you mean by "from the other side", but here is how I understand it:

    let intMatrix = fromString :: String -> Matrix Int

intMatrix is defined as a specialized version of the used-defined fromString, having type String -> Matrix Int.

    it "extract first row" $ do
      row 0 (intMatrix "1 2\n10 20") `shouldBe` vector [1, 2]

Knowing the type of intMatrix and row, the compiler can directly deduce that the expression on the left side of shouldBe is of type Vector Int, which is the same type as the right side.

I also don't feel it makes much sense in the context of the problem.

Yeah! It feels weird there!


It seems that fromString is there since the original commit that added the exercise.

I agree that there are reasons to remove it:

  • It isn't needed for the exercise to make sense.
  • It doesn't seem to add anything interesting to the exercise.
  • Exercises should probably be focused in a few concepts.
  • There seems to be no sensible implementation that isn't a partial function.

The only problem is that it isn't possible to remove fromString without rewriting the entire test suite, which may be a good idea...

@petertseng
Copy link
Member

Did the exercise, doing the string cases took more time than the entire rest of the problem, I think. Then again, I used the lowest-effort solution possible for the rest of the problem. reads skills not strong enough, apparently. It was an OK thing to think about, but good for its own exercise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants