Skip to content

high-scores: validate that functions do not modify the list data#1908

Closed
lekaf974 wants to merge 1 commit intoexercism:masterfrom
lekaf974:add-test-method-do-not-modify-list-data
Closed

high-scores: validate that functions do not modify the list data#1908
lekaf974 wants to merge 1 commit intoexercism:masterfrom
lekaf974:add-test-method-do-not-modify-list-data

Conversation

@lekaf974
Copy link

Update the test suite to validate that the solution used are not modifying the list data.
With this test the students will have to not use the methods that can modify the list like sort(), pop(), reverse(), etc..

@lekaf974 lekaf974 requested a review from a team as a code owner August 18, 2019 20:24
@yawpitch
Copy link
Contributor

Thank you for the PR, but I'm afraid I'm going to have to close this. As discussed in #1874, #1873, #1868, #1745, and #1735 we are not going require the student to avoid mutation of the list passed to the various functions.

@yawpitch yawpitch closed this Aug 18, 2019
@lekaf974
Copy link
Author

Ok,

I did not read this previous PRs

Regards.

@yawpitch
Copy link
Contributor

Sorry about the wasted effort. We updated the mentor notes to reflect this, but we really need a better way of communicating this decision other than closed PRs. Unfortunately Github doesn't really provide a great mechanism for this.

@cmccandless
Copy link
Contributor

@yawpitch Perhaps we could leverage issue templates for that?

@yawpitch
Copy link
Contributor

@cmccandless we could try, but most of the PRs we've ended up closing have been ones created by people who didn't created an associated Issue first. They also didn't do a search of the closed PRs... I tend to think neither an Issue / PR Template nor a Pinned Issue will get any more attention.

What I had considered is whether or not we should simply add a track-specific comment along the lines of "## NOTE: DO NOT IMPLEMENT TESTS FOR MUTATION" to the generated test file.

Another way to look at this is that perhaps we've got it wrong and we should ban mutation by informing the student that they should return a copy of the original list. After all mutation is a fundamental issue the instant you introduce the list type... maybe we shouldn't keep trying to pretend otherwise.

@cmccandless
Copy link
Contributor

Another way to look at this is that perhaps we've got it wrong and we should ban mutation by informing the student that they should return a copy of the original list. After all mutation is a fundamental issue the instant you introduce the list type... maybe we shouldn't keep trying to pretend otherwise.

Without endorsing one direction or the other: that would certain please several mentors who have raised this issue in the past.

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

Successfully merging this pull request may close these issues.

3 participants