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:
- split the input string
- encode all of the string pieces
- 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).