Skip to content

[run-length-encoding] test case conflicts with description #537

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
daveyarwood opened this issue Feb 6, 2017 · 8 comments
Closed

[run-length-encoding] test case conflicts with description #537

daveyarwood opened this issue Feb 6, 2017 · 8 comments

Comments

@daveyarwood
Copy link
Contributor

The description of the run-length-encoding exercise says:

For simplicity, you can assume that the unencoded string will only contain the letters A through Z.

However, there is one test case that contains spaces and lowercase letters.

I believe the test case should be changed to something like OEJJJJJXAAPPPL, the intent being that all of the test cases deal with strings containing only uppercase letters. I could make a PR to fix this test case, if you'd like?

@robkeim
Copy link
Contributor

robkeim commented Feb 6, 2017

Thanks @daveyarwood for pointing this out!

I added that description trying to fix #238, but as you pointed out my description was not complete. I think the correct thing to do would be to update the description to say something like:
"only letters A through Z (upper and lower case) as well as white space".

@daveyarwood
Copy link
Contributor Author

I wonder if that would create more work for the individual language repos? The example solutions would all need to be updated to handle spaces in the input.

@robkeim
Copy link
Contributor

robkeim commented Feb 6, 2017

Tracks that have already implemented this exercise should have been taking the existing whitespace into account (assuming that they're following the canonical data). Therefore, I think it makes more sense to not modify the canonical data and to update the description.

@daveyarwood
Copy link
Contributor Author

Ah, that's a good point. 👍

@robkeim
Copy link
Contributor

robkeim commented Feb 6, 2017

If you'd like to send a PR with an updated description go right ahead, otherwise let me know and I'll give it another shot.

@daveyarwood
Copy link
Contributor Author

I'm happy to take a stab at it.

Question, though: what about the lowercase z's? Are we considering valid input to be only uppercase letters and whitespace?

@robkeim
Copy link
Contributor

robkeim commented Feb 7, 2017

My understanding, given the current canonical data, is that we're accepting all ASCII characters except for the numbers, although the tests only test A through Z both upper and lowercase.

Here's another attempt (tell me what you think):

For simplicity, you can assume that the unencoded string only contains whitespace and the letters A through Z both upper and lowercase.

@daveyarwood
Copy link
Contributor Author

That sounds good to me 👍

I'll make a PR soon to update the description when I have a little bit of free time.

emcoding pushed a commit that referenced this issue Nov 19, 2018
Replaced private attr_reader methods with direct instance variable
access. This is not as elegant as the attr_reader version, but does not
generate interpreter warnings.

The warnings were:
```
xruby/lib/tasks/exercise_tests_runner.rb:23: warning: private attribute?
xruby/lib/tasks/exercise_tests_runner.rb:23: warning: private attribute?
xruby/lib/tasks/exercise_test_tasks.rb:24: warning: private attribute?
xruby/lib/tasks/exercise_test_tasks.rb:24: warning: private attribute?
```

These warnings are present in Ruby versions < 2.3.0
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

2 participants