Skip to content

Conversation

@petertseng
Copy link
Member

The READMEs now have 1-based indices but the tests still have 0-based.

Before these can be merged, must decide between various solutions:

  1. Change the tests to be one-indexed.
  2. Maintain our own README with zero-based indices. You do this by placing the README we want in .meta/description.md. Not recommended.
    • We cannot just use hints.md since that only allows additive changes. This is a modification of existing text, which hints.md does not support.
  3. Keep both the README and tests as is, but insert a statement somewhere (hints.md, or the tests) telling students to be aware of the discrepancy.
  4. Suggest to problem-specifications that no, really, they should go back to using zero-based indices.
  5. Your suggestion here.

@petertseng
Copy link
Member Author

My own opinion: Indifferent between 1 and 3.
I will not stop someone from doing 4, but it is not high value enough to be a movement I want to drive myself.
I do not think you can convince me to do 2.

@sshine
Copy link
Contributor

sshine commented Sep 3, 2019

Matrix: I wonder what kind of solution we'd like to recommend for 1-indexing. Incidentally, Data.Matrix.getElem uses 1-indexing.

For the current example solution using Data.Vector it seems that we'll be doing some translation between 0- and 1-indexing. An alternative is to use a Data.Array which supports arbitrary indexing with the Ix type class, but we'd implement a matrix using a library that is much closer to a matrix already.

Saddle-Points: We already use Data.Array here (both in the stub and in the example), so making the solution work with 1-indices in a nice way should be less of a hassle.

I prefer 1, but I think you should fix the example solutions however you like.

Matrix indices are usually defined so that (1, 1) is the top left entry,
not (0, 0). See for example any linear algebra textbook or wikipedia:
https://en.wikipedia.org/wiki/Matrix_(mathematics)#/media/File:Matrix.svg

exercism/problem-specifications#1387

Matches Data.Matrix.getElem:
https://hackage.haskell.org/package/matrix/docs/Data-Matrix.html#v:getElem
@petertseng petertseng changed the title matrix, saddle-points: Use 1-based indices matrix 1.1.0.6: Use 1-based indices Sep 4, 2019
@petertseng
Copy link
Member Author

Very well. It was not too hard to translate between 1-based and 0-based in the example implementation. This PR now only does matrix for reasons related to how I think things should be squashed.

@sshine sshine merged commit da85faa into exercism:master Sep 7, 2019
@petertseng petertseng deleted the readme-1based branch September 7, 2019 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants