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: Output chunks as a list of strings, not a single string #1827

Closed
wants to merge 1 commit into from
Closed

crypto-square: Output chunks as a list of strings, not a single string #1827

wants to merge 1 commit into from

Conversation

petertseng
Copy link
Member

@petertseng petertseng commented Aug 1, 2021

Problem:
Currently, the space character performs two duties in the output:

  1. Separates chunks
  2. Pads chunks that are shorter than others

This causes the output to be unnecessarily unclear about which of these
two roles any given space character represents.

Note that this would also cause code that wished to separate the
space-separated string into its chunks to be slightly awkward. However
this problem is not important to this Exercism exercise, because
students are not asked to write that chunking code.

Solution:
The unclarity can be resolved by instead outputting the chunks as a list
of strings. This requires replacing each of the existing test cases.

Recommendation for reviewers:
The author of this commit personally thinks pursuing this this is not
worth our volunteer maintainers' valuable time, and will close this PR
immediately upon submitting it. The author's only aim in submitting this
PR is as a record of this proposal and as an avenue for discussion.
However, this PR is ready to reopen and merge if it is decided that it
is worth pursuing.

The author of this commit thinks it's not worth pursuing this because
this is a ciphering exercise and therefore by nature the focus is
not on clarity for a human reader of the output.

Appendix:
Here is how other ciphering exercises treat their outputs:

  • affine-cipher: chunks of five, space-separated, no padding on last
    chunk
  • atbash cipher: chunks of five, space-separated, no padding on last
    chunk
  • rail-fence-cipher: neither inputs nor outputs have spaces
  • rotational-cipher: spaces present in plaintext are preserved exactly
    and appear in ciphertext in the corresponding position
  • simple-cipher: neither inputs nor outputs have spaces

@petertseng petertseng closed this Aug 1, 2021
@petertseng petertseng reopened this Aug 1, 2021
**Problem**:
Currently, the space character performs two duties in the output:

1. Separates chunks
2. Pads chunks that are shorter than others

This causes the output to be unnecessarily unclear about which of these
two roles any given space character represents.

Note that this would also cause code that wished to separate the
space-separated string into its chunks to be slightly awkward. However
this problem is not important to this Exercism exercise, because
students are not asked to write that chunking code.

**Solution**:
The unclarity can be resolved by instead outputting the chunks as a list
of strings. This requires replacing each of the existing test cases.

**Recommendation for reviewers**:
The author of this commit personally thinks pursuing this this is not
worth our volunteer maintainers' valuable time, and will close this PR
immediately upon submitting it. The author's only aim in submitting this
PR is as a record of this proposal and as an avenue for discussion.
However, this PR is ready to reopen and merge if it is decided that it
is worth pursuing.

The author of this commit thinks it's not worth pursuing this because
this is a ciphering exercise and therefore **by nature** the focus is
not on clarity for a human reader of the output.

**Appendix**:
Here is how other ciphering exercises treat their outputs:

* affine-cipher: chunks of five, space-separated, no padding on last
  chunk
* atbash cipher: chunks of five, space-separated, no padding on last
  chunk
* rail-fence-cipher: neither inputs nor outputs have spaces
* rotational-cipher: spaces present in plaintext are preserved exactly
  and appear in ciphertext in the corresponding position
* simple-cipher: neither inputs nor outputs have spaces
@petertseng petertseng closed this Aug 1, 2021
@SleeplessByte
Copy link
Member

Did you close this on purpose?

@petertseng
Copy link
Member Author

Yes, I closed it on purpose.

Perhaps closing it biases the discussion by predisposing reviewers towards keeping it closed and not pursing solving the problem, but I think it was better for me to close it for these reason:

  1. Given the work it may potentially cause for maintainers, the problem+solution had better be worth the time+effort it takes for them.
  2. Given the above + my own assessment of the problem, I think a safer default is to default to not pursuing this PR, to respect maintainers' valuable time. Closing it is the way to signify that I think this should be the default.

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.

2 participants