Skip to content

this is a proposal for the affine-cipher to replace the atbash problem #173

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
wants to merge 6 commits into from
Closed

this is a proposal for the affine-cipher to replace the atbash problem #173

wants to merge 6 commits into from

Conversation

guygastineau
Copy link
Contributor

Hi team,
I recently talked with budmc about adding tests for pipes and reading from a file to the atbash-cipher problem in the bash track. Since then I completed a bonafide Affine cipher and migrated the test.sh and README to accommodate an affine cipher instead of atbash. I think the affine cipher poses more interesting and challenging problems to the leanrer and would therefore be an excellent substitution for the atbash problem in the track. However, I will also admit that the atbash problem prepared me for dealing with the more complex affine cipher. Thus, while I suggest this to replace the atbash problem, I don't have a problem with them both existing in the track. I just didn't know if you all thought they would be too similar. I have included my script as example which will pass all of the tests so you all could play with the commit without having to do the problem yourselves. I am happy with my solution except for the function to check for the coprimality of a and m. I'm not really sure if there is a cleaner way to do it but it works for now.
Note: It tests to see if the solution can read from a file as an argument by comparing:
[[ "$(affine encode key affine_cipher_test.sh)" = "$(cat affine_cipher_test.sh | affine encode key)" ]]
It seemed silly to me to compare the output of the same problem, but since proper encoding and piping tests have been done before this test I figured it was sane enough. There is no guarantee that someone could not bug their way through that specific test though. I believe all of the other tests are fine. Unfortunately, bats gets the status as 2 when pipes are used after run, so I couldn't test [ "$status" = 0 ] on the test cases using pipes. Thank you for your consideration of my changes.


Reviewer Resources:

Track Policies

@guygastineau
Copy link
Contributor Author

I just realized I left my backup test in there as well as well in addition to not editing the config.json. I can fix those things later today and submit a new pull request.
Question: would you all like me to add it after Roman numerals in the config.json?

@guygastineau
Copy link
Contributor Author

Sorry for the flurry of commits. I am new to json. Fixed all but the fact that it doesn't have a uuid. I have no idea what the uuid should be.

@guygastineau
Copy link
Contributor Author

I did finally learn about configlet. I am attempting to install it.

@guygastineau
Copy link
Contributor Author

Finally got past Travis. I felt like it was more appropriate to add affine-cipher at the end with atbash-cipher as a prerequisite. I defer entirely to the maintainers judgement regarding how it should be folded into the existing exercises. Thank you for taking the time to review my submission.

@budmc29
Copy link
Member

budmc29 commented Jul 13, 2018

Hello @guygastineau, thanks for submitting this pull requests.

When we discussed I thought you wanted to implement an existing exercise to our bash track, but looks like I misunderstood, sorry about that.

For adding new exercises to exercism, we need to use: https://github.com/exercism/problem-specifications.

Please open a new issue here: https://github.com/exercism/problem-specifications/issues, and we can discuss about that.

Here is an example of somebody doing just that: exercism/problem-specifications#906

I'll close this PR since we can't merge it, but we can reopen it when the problem-specification will have this exercise.

Let me know if you need any help.

@budmc29 budmc29 closed this Jul 13, 2018
@guygastineau
Copy link
Contributor Author

guygastineau commented Jul 13, 2018 via email

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