Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

##The Algorithm##

The Algorithm

When using algorithms, it is helpful to name things accordingly so others can more easily understand what is going on. Even a novel algorithm can be somewhat categorized.

Your encryption algorithm is a substitution cipher, more specifically a monoalphabetic cipher. This means that for any given character m there is a new character c which substitutes it. A famous example of a monoalphabetic cipher is the Caesar cipher which creates the substitution alphabet by shifting the original alphabet.

Looking through your algorithm, I would categorize your algorithm as a Caesar cipher with a slight variation caused by this if:

if position < len(letters) - verplaatsing:
 position += verplaatsing
elif position > len(letters) - verplaatsing:
 position -= verplaatsing

Depending on the character you shift to right or the left. But this causes a problem, the small portion that is shifted to the left overlaps with the rest and causes different m to be substituted with the same c which means your cipher does not work. For example when encrypting #$%&'()*+,-./:;=>?@[\]^_`{|\~ with key 7 we get *+,-./:;<=>?@[\^_`{|}~ _?@[}], notice the duplicate ?@[ part.

As a side note, there is a bug in your decryption, which I found while testing this edge case. The condition in the if should be position < len(letters) + key - 1 with a + in front of the key because you are shifting in the other direction.

It seems like you wanted to implement a Caesar cipher but were unsure how to handle the case where you are shifting beyond your letter array. For a Caesar cipher you can simply use modulo:

index = letters.index(message)
newindex = (index + key) % len(letters)
cipher = letters[newindex]

For values where index + key < len(letters) nothing changes, but for values greater you wrap around to the beginning. This also simplyfies your encryption and has no out of range error.

In general do not roll you own crypto. When experimenting go nuts with your ideas but do not use it anywhere.

#Programming Stuff#

Programming Stuff

You do not need a while loop for iterating over a list. The idiomatic way is:

for cijfer in tekstpos:
 ontsleuteld.append(letters[cijfer])

When you need a counter the idiomatic way would be:

for x in range(len(tekst)):
 position = letters.index(tekst[x])
 if position < len(letters) - key - 1:
 position -= key
 elif position >= len(letters) - key - 1:
 position += key
 tekstpos.append(position)

Where range(n) creates a list of the form [0, 1, 2, ..., n-1]. These changes help limit your variables to a narrower scope so that they do not clutter the code.

##The Algorithm##

When using algorithms, it is helpful to name things accordingly so others can more easily understand what is going on. Even a novel algorithm can be somewhat categorized.

Your encryption algorithm is a substitution cipher, more specifically a monoalphabetic cipher. This means that for any given character m there is a new character c which substitutes it. A famous example of a monoalphabetic cipher is the Caesar cipher which creates the substitution alphabet by shifting the original alphabet.

Looking through your algorithm, I would categorize your algorithm as a Caesar cipher with a slight variation caused by this if:

if position < len(letters) - verplaatsing:
 position += verplaatsing
elif position > len(letters) - verplaatsing:
 position -= verplaatsing

Depending on the character you shift to right or the left. But this causes a problem, the small portion that is shifted to the left overlaps with the rest and causes different m to be substituted with the same c which means your cipher does not work. For example when encrypting #$%&'()*+,-./:;=>?@[\]^_`{|\~ with key 7 we get *+,-./:;<=>?@[\^_`{|}~ _?@[}], notice the duplicate ?@[ part.

As a side note, there is a bug in your decryption, which I found while testing this edge case. The condition in the if should be position < len(letters) + key - 1 with a + in front of the key because you are shifting in the other direction.

It seems like you wanted to implement a Caesar cipher but were unsure how to handle the case where you are shifting beyond your letter array. For a Caesar cipher you can simply use modulo:

index = letters.index(message)
newindex = (index + key) % len(letters)
cipher = letters[newindex]

For values where index + key < len(letters) nothing changes, but for values greater you wrap around to the beginning. This also simplyfies your encryption and has no out of range error.

In general do not roll you own crypto. When experimenting go nuts with your ideas but do not use it anywhere.

#Programming Stuff#

You do not need a while loop for iterating over a list. The idiomatic way is:

for cijfer in tekstpos:
 ontsleuteld.append(letters[cijfer])

When you need a counter the idiomatic way would be:

for x in range(len(tekst)):
 position = letters.index(tekst[x])
 if position < len(letters) - key - 1:
 position -= key
 elif position >= len(letters) - key - 1:
 position += key
 tekstpos.append(position)

Where range(n) creates a list of the form [0, 1, 2, ..., n-1]. These changes help limit your variables to a narrower scope so that they do not clutter the code.

The Algorithm

When using algorithms, it is helpful to name things accordingly so others can more easily understand what is going on. Even a novel algorithm can be somewhat categorized.

Your encryption algorithm is a substitution cipher, more specifically a monoalphabetic cipher. This means that for any given character m there is a new character c which substitutes it. A famous example of a monoalphabetic cipher is the Caesar cipher which creates the substitution alphabet by shifting the original alphabet.

Looking through your algorithm, I would categorize your algorithm as a Caesar cipher with a slight variation caused by this if:

if position < len(letters) - verplaatsing:
 position += verplaatsing
elif position > len(letters) - verplaatsing:
 position -= verplaatsing

Depending on the character you shift to right or the left. But this causes a problem, the small portion that is shifted to the left overlaps with the rest and causes different m to be substituted with the same c which means your cipher does not work. For example when encrypting #$%&'()*+,-./:;=>?@[\]^_`{|\~ with key 7 we get *+,-./:;<=>?@[\^_`{|}~ _?@[}], notice the duplicate ?@[ part.

As a side note, there is a bug in your decryption, which I found while testing this edge case. The condition in the if should be position < len(letters) + key - 1 with a + in front of the key because you are shifting in the other direction.

It seems like you wanted to implement a Caesar cipher but were unsure how to handle the case where you are shifting beyond your letter array. For a Caesar cipher you can simply use modulo:

index = letters.index(message)
newindex = (index + key) % len(letters)
cipher = letters[newindex]

For values where index + key < len(letters) nothing changes, but for values greater you wrap around to the beginning. This also simplyfies your encryption and has no out of range error.

In general do not roll you own crypto. When experimenting go nuts with your ideas but do not use it anywhere.

Programming Stuff

You do not need a while loop for iterating over a list. The idiomatic way is:

for cijfer in tekstpos:
 ontsleuteld.append(letters[cijfer])

When you need a counter the idiomatic way would be:

for x in range(len(tekst)):
 position = letters.index(tekst[x])
 if position < len(letters) - key - 1:
 position -= key
 elif position >= len(letters) - key - 1:
 position += key
 tekstpos.append(position)

Where range(n) creates a list of the form [0, 1, 2, ..., n-1]. These changes help limit your variables to a narrower scope so that they do not clutter the code.

Source Link
henje
  • 342
  • 1
  • 6

##The Algorithm##

When using algorithms, it is helpful to name things accordingly so others can more easily understand what is going on. Even a novel algorithm can be somewhat categorized.

Your encryption algorithm is a substitution cipher, more specifically a monoalphabetic cipher. This means that for any given character m there is a new character c which substitutes it. A famous example of a monoalphabetic cipher is the Caesar cipher which creates the substitution alphabet by shifting the original alphabet.

Looking through your algorithm, I would categorize your algorithm as a Caesar cipher with a slight variation caused by this if:

if position < len(letters) - verplaatsing:
 position += verplaatsing
elif position > len(letters) - verplaatsing:
 position -= verplaatsing

Depending on the character you shift to right or the left. But this causes a problem, the small portion that is shifted to the left overlaps with the rest and causes different m to be substituted with the same c which means your cipher does not work. For example when encrypting #$%&'()*+,-./:;=>?@[\]^_`{|\~ with key 7 we get *+,-./:;<=>?@[\^_`{|}~ _?@[}], notice the duplicate ?@[ part.

As a side note, there is a bug in your decryption, which I found while testing this edge case. The condition in the if should be position < len(letters) + key - 1 with a + in front of the key because you are shifting in the other direction.

It seems like you wanted to implement a Caesar cipher but were unsure how to handle the case where you are shifting beyond your letter array. For a Caesar cipher you can simply use modulo:

index = letters.index(message)
newindex = (index + key) % len(letters)
cipher = letters[newindex]

For values where index + key < len(letters) nothing changes, but for values greater you wrap around to the beginning. This also simplyfies your encryption and has no out of range error.

In general do not roll you own crypto. When experimenting go nuts with your ideas but do not use it anywhere.

#Programming Stuff#

You do not need a while loop for iterating over a list. The idiomatic way is:

for cijfer in tekstpos:
 ontsleuteld.append(letters[cijfer])

When you need a counter the idiomatic way would be:

for x in range(len(tekst)):
 position = letters.index(tekst[x])
 if position < len(letters) - key - 1:
 position -= key
 elif position >= len(letters) - key - 1:
 position += key
 tekstpos.append(position)

Where range(n) creates a list of the form [0, 1, 2, ..., n-1]. These changes help limit your variables to a narrower scope so that they do not clutter the code.

lang-py

AltStyle によって変換されたページ (->オリジナル) /