Skip to content

Edit README to clarify exercise instructions (crypto-square) #204

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

Merged
merged 1 commit into from
May 8, 2016
Merged

Edit README to clarify exercise instructions (crypto-square) #204

merged 1 commit into from
May 8, 2016

Conversation

matthewmorgan
Copy link
Contributor

This PR is to address #190 .

@petertseng asked if the output should be chunked. My opinion, shared with @jtigger, is that it should be, as it was in the original exercise that inspired this problem, and chunking the output fits aesthetically with the idea of creating a visual 'square' which is what the original users of this method would have done.

We're hoping we can get some consensus on the content of the README, then as a next step add a config.json for this exercise. Thanks all!

@petertseng
Copy link
Member

I think these are great changes because they make clear the that the decision to chunk the output is a separate step from actually figuring out what the letters in the output should be. Further it explains the rationale for the chunk size that we chose. Thanks for taking this on.

Do you anticipate that removing the examples of 4 characters -> 2x2, 81 characters -> 9x9, 5-8 characters -> 3 columns 2 rows will cause students to have a hard time figuring out the best way to compute them? From the original text, it was quick to see that finding the smallest number whose square is >= the message size would do it (alternatively, ceil(sqrt(message_size))). Is the new challenge a good or bad thing?

It seems that "print out in chunks of r letters" would misalign some letters. If you look at the letters in the README, I can decode "If man was meant to..." and then after that the letters get mistaligned with "etay on tho ground gad would huve given srootss". The page at http://users.csc.calpoly.edu/~jdalbey/103/Projects/ProgrammingPractice.html seems to have a slightly different chunking of "imtgdvs fearwer mayoogo anouuio ntnnlvt wttddes aohghn sseoau" - the difference being that the chunks are of size 7, 7, 7, 7, 7, 7, 6, 6, rather than 7, 7, 7, 7, 7, 7, 7, 5. That would stack up to form:

imtgdvs
fearwer
mayoogo
anouuio
ntnnlvt
wttddes
aohghn 
sseoau

from which the letters do get aligned to form the original message.

So we can either match their scheme (which would require adjusting our explanation slightly to match) or stick with this and clarify that some letters will get shifted around.

To see the effects of this decision we can also, we can look at exercism/exercism#2432 (comment) for examples of chunking a 26 character string in either 5, 5, 4, 4, 4, 4 or 5, 5, 5, 5, 5, 1 (in the latter, the letters get shifted around even more drastically, such that you can't really discern the plaintext just by stacking them up)

@matthewmorgan
Copy link
Contributor Author

@petertseng you're absolutely right about the letter alignment. Good catch.

It appears the examples in the original (calpoly) exercise distribute the n unfilled spaces across the final n rows of the square. Do I have that right? If so, then we should change the README to reflect that.

As far as omitting the extra examples, I think that one well-written example is OK. We just have to get this one there!

@jtigger
Copy link
Contributor

jtigger commented Mar 14, 2016

Yup, and the .json will inform unit tests that should help convey these corner cases.

(sent from mobile)

@petertseng
Copy link
Member

It appears the examples in the original (calpoly) exercise distribute the n unfilled spaces across the final n rows of the square. Do I have that right?

I agree with this.

Output the encoded text in chunks. Phrases that fill perfect squares
`(r X r)` should be output in `r`-length chunks separated by spaces.
Imperfect squares should distribute the `n` empty spaces across the last
`n` rows of the square by making the length of those final `n` rows `c-1`.
Copy link
Member

Choose a reason for hiding this comment

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

r - 1 here or c - 1? I think it's r - 1 but check me on that

@matthewmorgan
Copy link
Contributor Author

Yep. I just noticed that row and column counts are swapped in the 'stacked' chunks.

Thanks again @petertseng !

@petertseng
Copy link
Member

Yep. I just noticed that row and column counts are swapped in the 'stacked' chunks.

Ah that's right, that's why I too got a bit mixed up. Probably best if we keep just one value of r throughout the readme (as in, what I see now is right, with r - 1 there).

So personally I would be 👍 on this. If anyone else wishes to chime in?

Thanks again for the work on this.

@kytrinyx
Copy link
Member

This is much clearer, thanks so much for sorting through this! I'm happy with this as well.

I'm about to get on a plane for a ridiculously long trip. If nobody else has chimed in by the time I get to my destination (40 hours), then I'll go ahead and merge it then.

@jtigger
Copy link
Contributor

jtigger commented Mar 17, 2016

Hey @kytrinyx, do you have a process by which you stage up updates to existing exercises such that the README and the various track implementations deploy together?

Do we need to create issues in each track that will now have a delta from the README?

@kytrinyx
Copy link
Member

Sorry it took so long to get to this... I've been traveling and have barely checked github (or email) for the past 5 days.

I don't have a process; sounds like I probably should, though. See exercism/discussions#2 for a brand new discussion about it.

If one of you comes up with a suggested text for the issue, I can cross-post it to all the tracks that have the crypto-square exercise in it.

@matthewmorgan
Copy link
Contributor Author

At this point I'm planning on creating a crypto-square.json to flesh out this PR. @jtigger and I discussed yesterday and it seems like it would be a good idea.

@kytrinyx
Copy link
Member

That sounds excellent!

@kotp kotp changed the title Edit README to clarify exercise instructions Edit README to clarify exercise instructions (crypto-square) Apr 27, 2016
@IanWhitney
Copy link
Contributor

I'm going to start implementing crypto-square for Rust soon. Can this PR be merged? And @matthewmorgan, how is the json file coming along? Anything I can do to help?

@kytrinyx
Copy link
Member

kytrinyx commented May 8, 2016

Yeah, let's merge this and have a separate PR for the JSON file.

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.

5 participants