[Problem Specifications - Matrix] Add new test case

Based on conversation with @senekor on this PR.

I wanted to get some feedback on potentially adding 1-2 more test cases to the Matrix exercise.

Currently every test case only gives valid input to the row and column methods/functions required by the challenge. Interestingly, there are two tests which imply an invalid input:

  1. “can extract row from non-square matrix with no corresponding column”
  2. “can extract column from non-square matrix with no corresponding row”

However, the inverse of either of the above is never actually tested. I suggest something to the effect of -

{
 ...
 "description": "cannot extract row with no corresponding row in matrix"
 "property": "row",
 "input": {
  "string": "1 2 3\n4 5 6\n7 8 9",
  "index": 4
 },
 "expected": null
}

The opposite may not be necessary. The main point would be to encourage the user to implement code that considers edge cases which is important in any language.

Canonical Data - Link

We’re removed a bunch of input validation and invalid inputs from various exercises in the past. Most exercises have a specific purpose/algorithm/goal in mind. Adding a bunch of input validation requirements to exercises where validation is not a goal adds a bunch of overhead which distracts from the goal. Invalid inputs are good for exercises that explicitly target validation. Invalid inputs are annoying and distracting in other exerises.

1 Like

Granted, good production code should validate all inputs always. And students are welcome to make their code robust. But it should not be a requirement.

Invalid inputs are good for exercises that explicitly target validation. Invalid inputs are annoying and distracting in other exerises.

I do absolutely agree with you there. Most exercises would not benefit from the additional annoyance and it wouldn’t really teach the student much. I still would debate that it would be beneficial for this exercise, however.

I’m sure you know as well as I do that grid/matrix type problems are a very common coding challenge. Usually in these challenges the biggest issue isn’t the input as much as it is the coder. I have failed so many tests because I didn’t put something to the effect of if y < 0 || x >= length of grid[0] ... (or whatever) in my code at some point. It just seems to make sense with this challenge to add in the additional problem/skill of dealing with the boundaries.

With that being said, I’m relatively new here. I respect your point and you’ve probably dealt with a lot more user feedback than I have. I’ll defer to those with more experience than me :slight_smile:

You may want to also read Suggesting Exercise Improvements | Exercism's Docs

I read it over and it said pretty much exactly what you said. I appreciate you sharing that with me and I’ll keep that in mind for the future.

Thanks!

Isaac is correct in that we have actually removed a lot of input validation test cases, as there’s only limited value in doing input validation over and over again as well as the fact that we’re not aiming for production grade code.

That said, I do see the argument for adding the proposed test cases, as working with matrices and boundary conditions is so incredibly common. We could add them with the input-validation scenario to allow test generators to easily opt-out.

Maybe some other people could chime in?

While scenarios let tracks opt out, that doesn’t solve the input validation problem nor invalidating existing solutions. The scenarios are helpful for things like Unicode support or edge cases that some languages don’t have good support for. Using scenarios to gate input validation feels like it defeats the purpose of having a canonical set of tests and defers to track maintainers the decision of what tests an exercise should have.

You definitely make a lot of valid points. If this is a test that most maintainers/contributors would end up opting out of, it really does not make sense for it to be included in the tests overall.

What it really comes down to is if an input validation type test belongs in this challenge or not. I would be curious to hear more thoughts as well.

In the meantime, we just opted to add some additional test to the Rust track’s version and go from there.

1 Like