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

Unit tests for Cryto Square problem don't match problem description #149

Closed
wboayue opened this issue Oct 4, 2016 · 4 comments
Closed

Comments

@wboayue
Copy link

wboayue commented Oct 4, 2016

Per problem description: https://github.com/exercism/x-common/blob/master/exercises/crypto-square/description.md

The number of columns should be greater than or equal to the number of rows. However, the java unit tests expect a solution with the number of rows is greater than or equal to the number of columns.

@jtigger
Copy link
Contributor

jtigger commented Oct 4, 2016

what the....??!? @matthewmorgan, is this related to #55?

Thanks for reporting this, @wboayue !

@matthewmorgan
Copy link
Contributor

Hmm...

TLDR; the crypto output is expected to be a different format than the plaintext output. I believe that the tests are correct.

I think this might just be a misunderstanding of the problem requirements, probably due to insufficient explanation in the README.

The initial mapping of the plaintext is specified:

The plaintext should be organized in to a rectangle. The size of the rectangle (r x c) should be decided by the length of the message, such that c >= r and c - r <= 1, where c is the number of columns and r is the number of rows.

So, yes, the plaintext should be r X c with c >=r, but c shouldn't be more than 1 greater than r. The example plain text is 7 rows of 8 columns.

The crypto text output could (maybe?) use a better explanation. The example shows 8 rows of 7 columns (swapped from the original dimensions) with the final column empty in the last few rows. The reason for this is the 'visual' presentation of the crypto-square.

As I recall, we opted not to provide a 'perfect square' and an 'imperfect square' explanation in the README, maybe it would be worth adding.

@jtigger @wboayue please let me know if you think I have misread the code or the issue.

@wboayue
Copy link
Author

wboayue commented Oct 4, 2016

@jtigger @matthewmorgan Thanks for looking into this. I misread the README. I inadvertently thought the constraint applied to both the plain and cipher text, but it makes sense that the constraints flips when you build the cipher text.

@wboayue wboayue closed this as completed Oct 4, 2016
@matthewmorgan
Copy link
Contributor

@wboayue no worries. I would be embarrassed to tell you how extended a discussion it required for us to get this right! I was hoping we hadn't made, after all that, another mistake. 😸

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

3 participants