Given this ciphertext from a Caesar cipher:


The task is to decrypt it without being given the key.

My solution:

napis = "fxeyaxklqxhkltkxqebobxtbobxplxjykvxfaflqpxfkxqebxtloiaxrkqfixfxpqyoqbaxrpfkdxqebxfkqbokbq".upper()
napis = [ALPHABET.index(i) for i in napis]

for x in range(0,4):
    wynik = [ALPHABET[i+x] if i+x<len(ALPHABET) else ALPHABET[i+x-len(ALPHABET)] for i in napis]


  • \$\begingroup\$ @Toby My answer's suggestion with frequencies and spaces was inspired/justified by that now-deleted sentence :-( \$\endgroup\$ Feb 3, 2021 at 17:40
  • \$\begingroup\$ Oh, sorry - it looked like a request to change the code. Is there a way it could be fixed and reinstated? \$\endgroup\$ Feb 3, 2021 at 18:25
  • 1
    \$\begingroup\$ Welcome to Code Review! Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see What should I do when someone answers my question? as well as what you may and may not do after receiving answers. \$\endgroup\$ Feb 3, 2021 at 23:24

2 Answers 2

  • Don't use meaningless names like "napis" and "wynik".
  • x and i are meaningful in some contexts, but not for what you're using them, so use better names for those as well.
  • You could take advantage of negative indices, i.e., remove ALPHABET[i+x] if i+x<len(ALPHABET) else and just use ALPHABET[i+x-len(ALPHABET)]. Or, more generally, ALPHABET[(i+x) % len(ALPHABET)].
  • You could assume that spaces are the most frequent characters and do x = ALPHABET.index(' ') - max(napis, key=napis.count) instead of the loop.
  • 4
    \$\begingroup\$ Polish words are "meaningless" to you, but probably convey a lot more to the author (presumably Polish?). I don't think it's fair to insist that everyone code in English, even when the keywords are English, unless they intend to share that code amongst English speakers. It's not even that hard for the rest of us: Wiktionary says they mean string and result. \$\endgroup\$ Feb 3, 2021 at 17:02
  • 1
    \$\begingroup\$ @TobySpeight They did share that code amongst English speakers. \$\endgroup\$ Feb 3, 2021 at 17:03
  • 3
    \$\begingroup\$ It might be better to say "Use English names" rather than calling Polish "meaningless". It just seems offensive to belittle an entire language like that. \$\endgroup\$ Feb 3, 2021 at 17:04
  • 1
    \$\begingroup\$ @TobySpeight I'm not belittling a culture. I see no meaning in those words and I even checked them at dictionary.com (because I'm not a native English speaker), and there are no results. They're asking for a review, i.e., what we think of their code, and that's what I think. \$\endgroup\$ Feb 3, 2021 at 17:09
  • 1
    \$\begingroup\$ Wiktionary evidently has better coverage: napis, wynik \$\endgroup\$ Feb 3, 2021 at 17:12

It seems like your current solution would require the person to look through all the possible decryptions and decide which one is right. That could work ok, but it might not be necessary. It could be possible for the program to figure it out itself.

Here’s an example of superb rain’s idea to assume the decryption with the most spaces is the correct one:

import string

encrypted_text = "fxeyaxklqxhkltkxqebobxtbobxplxjykvxfaflqpxfkxqebxtloiaxrkqfixfxpqyoqbaxrpfkdxqebxfkqbokbq"

ch_list = string.ascii_lowercase + ' '

def translation_maker(offset, ch_list):
    translation_dict = dict()
    for ind, ch in enumerate(ch_list):
        translation_dict[ch] = ch_list[(ind + offset) % len(ch_list)]
    return str.maketrans(translation_dict)

def translation_generator(text, ch_list):
    for ind in range(len(ch_list)):
        for offset in range(len(ch_list)):
            yield text.translate(translation_maker(offset, ch_list))
likely_decryption = max(translation_generator(encrypted_text, ch_list), key=lambda x: x.count(' '))

# a lot of this could be one-lined, but it might maybe considered less readable
# e.g.:
likely_decryption = max(
                ch: ch_list[(ind + offset) % len(ch_list)]
                for ind, ch in enumerate(ch_list)
        for offset in range(len(ch_list))
    key=lambda x: x.count(' '))

It prints the correct string in this case.

A different thing you could do is have a big set with every word in the English language, and assume the correct decryption is the one that has the most matches with the set after you split it by space. It would be mostly the same, but the lambda function would be changed to this: key=lambda x: sum(word in word_set for word in x.split()))

This might be slower even when disregarding the creation of the word set, and it would obviously use more memory, but it would be more unlikely to give a wrong result.


Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.