Skip to content
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

crypto-square: Clarifying how whitespace should be handled #415

Closed

Conversation

smaclell
Copy link

It is hip to be square

I was a little confused by the description for how extra whitespace should be handled in the crypto-square problem. Based on the tests in the xecmascript instead of evenly spacing the extras whitespace across multiple rows, the whitespace appeared to be added to the final row.

I have updated the wording to try to better reflect how the whitespace was handled. Thank you for such an awesome project and being so open to contributions.

@rbasso
Copy link
Contributor

rbasso commented Oct 19, 2016

Imperfect squares will have n empty spaces. Those spaces should be distributed evenly across the last n rows.

From my understanding, this is referring to the square after transposition.

If I'm not getting this wrong, this change will make the description incompatible with the current tests, right?

Copy link
Contributor

@Insti Insti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a modification rather than a clarification.
The correct thing to do would be to fix the apparently broken tests in the xecmascript track.

@@ -60,6 +62,6 @@ mayoogo
anouuio
ntnnlvt
wttddes
aohghn
sseoau
aohghns
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now reading the text top to bottom, it will be misaligned - before this change it was "if man was meant to stay on the ground god would have given us roots", and now it is "if man was meant to etay on tho ground gad would huve given s rootss" - is that okay? One can argue it is a measure to obscure the plain text, in which case we should just pick a group size unrelated to the square size, such as "always 5"

You can read previous discussion about it in #190, but it does appear the https://github.com/exercism/x-common/blob/master/exercises/crypto-square/canonical-data.json file matches the version before this PR rather than after it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am going to trust the canonical data and will instead go fix the specs in the ecmascript track. You are also correct that my description change would have misaligned the data. Thank you for responding.

@smaclell
Copy link
Author

Thanks everyone for your feedback! @Insti, I will take your suggestion and go submit a PR to fix the bad specs in the ecmascript track. Have a great day!

@smaclell smaclell closed this Oct 20, 2016
@smaclell smaclell deleted the clearer-wording-on-end-condition branch October 20, 2016 03:30
@Insti
Copy link
Contributor

Insti commented Oct 20, 2016

Great. Thanks for taking the initiative to try and fix the problem you found @smaclell

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.

4 participants