ROT13 encryption using array indexing

Question

This is a basic rot13 encryption program. I'm just looking for a general review, any feedback, or any optimization of what I wrote today as practice. I know that I need to not rely on indexes as I do in this code for iteration, but in this specific function I'm not sure how to get around it.

import string def encrypt(message): alpha = list(string.ascii_lowercase) mes = message.lower() indexes = [] for each in range(len(mes)): for i in range(len(alpha)): if alpha[i] == mes[each]: indexes.append(i + 1) result = [] for each in indexes: result.append(alpha[(each + 12) % len(alpha)]) return ''.join(result) def rot13(somestrings): check = somestrings.split(' ') result = [] for each in check: result.append(encrypt(each) + ' ') return ''.join(result)[:-1] 

Answer 1

There is already an example of better code serving your purpose, so here are some of the things that should be worked on in your code.

Document your code

Your code currently has no documentation, meaning the caller does not know what to expect when calling your code.

Your rot13 function:

• removes all characters other than ASCII letters and spaces
• converts letters to lower-case
• encodes what's left

This differs from what I would expect, but this is fine: as the author you get to chose the behavior of your function. As the caller, I get to choose to use that function or not. But in order to to that you need to communicate what I should expect from your code.

Python conventions recommend docstrings for this kind of documentation. In your case, it could read something like this:

def rot13(somestrings): """ Encode the input string using the ROT13 algorithm. The input string is stripped of all characters except ASCII letters and space characters, and ASCII letters are converted to lower case. All other characters are discarded Parameters ---------- somestrings : input string Returns ------- rot13 encoded string >>> rot13('abc') 'nop' >>> rot13('abc nop') 'nop abc' >>> rot13("FOO123 bar*ù") 'sbb one' """ # do stuff 

Better naming

In most cases, your variable names are not very descriptive, and sometimes straight-up misleading. For example, the input parameter for rot13 is called somestrings, which I would expect to be a collection of strings. However, rot13(['foo', 'bar']) raises an error.

Logic issues

It looks to me like both your rot13 and encrypt do the same thing:

1. split the input string
2. encode all of the string pieces
3. join the pieces together

There is no need to do this twice.

Magic numbers

You use unnamed constants, aka magic numbers, in your code. This can make it hard to read and follow along. In your case, that number is half the length of the English alphabet, and the issue is made worse as it is split into 1 + 12 in your encrypt function.

Iterate on content, not on indices

In Python, it is better practice on iterate on the content of an iterable rather than on indices. It usually makes the code cleaner and faster:

for letter in message: # do stuff with letter # don't do this unless you can't help it: for i in range(len(message)): # do stuff with message[i] 

Use built-in function

The whole inner for loop in encrypt can be replaced by:

string.ascii_lowercase.index(letter) 

Which is shorter, more readable, less error-prone, and most likely faster (or definitely faster in your case, as your inner loop doesn't exit early when a match is found).

Answer 2

You can achieve results more easily. In essence, displacement means cutting in half and swapping parts.

You will also need a dictionary for quick substitution, then you do not need an array and join

def rot13(message): alpha = string.ascii_lowercase omega = alpha[13:] + alpha[0: 13] # get with offset dictionary = {x: omega[i] for i, x in enumerate(alpha)} # make dict dictionary[' '] = ' ' # for space result = '' for letter in message.lower(): result += dictionary[letter] return result