-
-
Notifications
You must be signed in to change notification settings - Fork 46.6k
Improve readability of ciphers/mixed_keyword_cypher.py #8626
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
Changes from 5 commits
518f4e5
16c9268
a6d64ff
642ccc3
3520c16
3a099f5
738f193
b284ab1
ad436ce
dbbd778
be6f043
33b34c4
66e28ee
35d3de2
0bf2ed2
05d0283
ad8c99f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,9 @@ | ||
def mixed_keyword(key: str = "college", pt: str = "UNIVERSITY") -> str: | ||
""" | ||
from string import ascii_uppercase | ||
|
||
|
||
For key:hello | ||
def mixed_keyword(keyword: str = "college", plaintext: str = "UNIVERSITY") -> str: | ||
""" | ||
For keyword: hello | ||
|
||
H E L O | ||
A B C D | ||
|
@@ -19,50 +21,48 @@ def mixed_keyword(key: str = "college", pt: str = "UNIVERSITY") -> str: | |
'Y': 'T', 'Z': 'Y'} | ||
'XKJGUFMJST' | ||
""" | ||
key = key.upper() | ||
pt = pt.upper() | ||
temp = [] | ||
for i in key: | ||
if i not in temp: | ||
temp.append(i) | ||
len_temp = len(temp) | ||
# print(temp) | ||
alpha = [] | ||
modalpha = [] | ||
for j in range(65, 91): | ||
t = chr(j) | ||
alpha.append(t) | ||
if t not in temp: | ||
temp.append(t) | ||
# print(temp) | ||
r = int(26 / 4) | ||
# print(r) | ||
k = 0 | ||
for _ in range(r): | ||
s = [] | ||
for _ in range(len_temp): | ||
s.append(temp[k]) | ||
if k >= 25: | ||
break | ||
k += 1 | ||
modalpha.append(s) | ||
# print(modalpha) | ||
d = {} | ||
j = 0 | ||
k = 0 | ||
for j in range(len_temp): | ||
for m in modalpha: | ||
if not len(m) - 1 >= j: | ||
break | ||
d[alpha[k]] = m[j] | ||
if not k < 25: | ||
keyword = keyword.upper() | ||
plaintext = plaintext.upper() | ||
alphabet = list(ascii_uppercase) | ||
alphabet_set = set(alphabet) | ||
|
||
# create a list of unique characters in the keyword | ||
unique_chars = [] | ||
for char in keyword: | ||
if char in alphabet_set and char not in unique_chars: | ||
unique_chars.append(char) | ||
# the number of those unique characters will determine the number of rows | ||
num_unique_chars_in_keyword = len(unique_chars) | ||
|
||
# create a shifted version of the alphabet | ||
shifted_alphabet = unique_chars + [ | ||
char for char in alphabet if char not in unique_chars | ||
] | ||
|
||
# create a modified alphabet by splitting the shifted alphabet into rows | ||
modified_alphabet = [ | ||
shifted_alphabet[k : k + num_unique_chars_in_keyword] | ||
for k in range(0, 26, num_unique_chars_in_keyword) | ||
] | ||
|
||
# map the alphabet characters to the modified alphabet characters | ||
# going 'vertically' through the modified alphabet - consider columns first | ||
mapping = {} | ||
letter_index = 0 | ||
for column in range(num_unique_chars_in_keyword): | ||
for row in modified_alphabet: | ||
# if current row (the last one) is too short, break out of loop | ||
if len(row) <= column: | ||
break | ||
k += 1 | ||
print(d) | ||
cypher = "" | ||
for i in pt: | ||
cypher += d[i] | ||
return cypher | ||
|
||
# map current letter to letter in modified alphabet | ||
mapping[alphabet[letter_index]] = row[column] | ||
letter_index += 1 | ||
|
||
print(mapping) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ideally the algorithm shouldn't have side effects such as printing, but I understand that printing the mapping is helpful for debugging purposes. Perhaps it'd be better to have a "verbose" function parameter that allows users to enable/disable printing the mapping There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done - I've also added a test case that includes setting the verbose parameter to False |
||
# create the encrypted text by mapping the plaintext to the modified alphabet | ||
return "".join(mapping[char] if char in mapping else char for char in plaintext) | ||
|
||
|
||
print(mixed_keyword("college", "UNIVERSITY")) | ||
if __name__ == "__main__": | ||
print(mixed_keyword("college", "UNIVERSITY")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... plus other changes to remove references to
alphabet
I think converting
ascii_uppercase
to a list is unnecessary. Converting to a set, iterating, and indexing should all still work if you use the imported string directly.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can of course change that but I wanted to make it easier for people who would like to use other alphabets (e.g. cyrillic) for their ciphers. So it is your call - should I change it or not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If people want to use other alphabets such as Cyrillic, then they would have to go into the source code and change
ascii_uppercase
to a different variable. That doesn't really address the issue thatascii_uppercase
(or whatever the user replaces it with) still doesn't need to be converted to a list.If you want it to be easier to switch to a different alphabet, then perhaps you could have the function take in the alphabet string (
ascii_uppercase
, etc) as an input parameterThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair point - I've done both - Now there's no conversion to the list and a different alphabet can be passed to the function. Thank you for the review!