Encryption using a mirror field

This is my attempt at the mirror encryption challenge posted on r/dailyprogrammer/. The idea is that each character is to be replaced by its counterpart in the mirror field, as if a laser pointer were reflected off of all of the diagonal barriers in the grid.

Since I am relatively new to python, I'd like for someone to just have a look and point out some things that I need to look out for and where I can improve please.

```
class Encrypt(object):
"""
String encryption using a 13x13 mirror field
"""

mirrorField = (
('', 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', ''),
('A', '', '', '', '\\', '\\', '', '', '/', '\\', '', '', '', '', 'n'),
('B', '', '', '', '', '', '', '', '', '', '', '', '', '\\', 'o'),
('C', '', '', '', '/', '', '', '', '', '', '', '', '', '', 'p'),
('D', '', '', '', '', '', '', '\\', '', '', '', '', '', '\\', 'q'),
('E', '', '', '', '', '\\', '', '', '', '', '', '', '', '', 'r'),
('F', '', '', '/', '', '', '', '', '', '', '/', '', '', '', 's'),
('G', '\\', '', '', '/', '', '', '', '', '', '', '\\', '', '', 't'),
('H', '', '', '', '', '', '\\', '', '', '', '', '', '', '', 'u'),
('I', '\\', '/', '', '', '', '', '', '', '', '', '', '', '', 'v'),
('J', '/', '', '', '', '', '', '', '', '', '', '', '', '', 'w'),
('K', '', '', '', '', '', '', '', '', '', '', '\\', '', '', 'x'),
('L', '', '', '', '', '\\', '/', '', '', '', '', '', '', '', 'y'),
('M', '', '', '', '/', '', '', '', '', '', '', '', '/', '', 'z'),
('', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z', '')
)

def __init__(self):
self.direction

Your execution flow is very convoluted and unconventional. (The technical term for that is "spaghetti".) At first glance, it appears that you have defined a useful class and methods. However, on closer inspection, those "functions" are communicating with each other through variables (direction, encrypted, and running) that are somewhat global in nature.

Let's consider the instance variables.

encrypted is cleared in __init__(), built in check_mirror_position(), and output in start(). What if you want to encrypt another string using the same mirror field — what happens if you call start() again?

direction is initialized in __init__() (unnecessarily), reinitialized in start(), consulted in encrypt(), and reassigned in check_mirror_position().

running is initialized to False in __init__() (unnecessarily), reinitialized to True in start(), and tested in encrypt(), until it is set to False in check_mirror_position().

Consider the main code:

encrypt = Encrypt()
encrypt.start('TpnQSjdmZdpoohd')

What does "start" do? If there is a start, where does it finish? (Actually, start() does all of the work from start to finish!) Does it print anything, and if so, where is the print statement? What if you wanted to write unit tests or incorporate the encryption routine into a web application? You couldn't easily do it, since it's hard-wired to print its output to stdout. A well designed class would be used more like this:

encryptor = MirrorFieldEncryptor(grid)
print(encryptor.encrypt('TpnQSjdmZdpoohd'))

As you can see, I would have expected that the encrypt() method would be the one to call to encrypt a string. But that's not what your encrypt() method does. Rather, it accepts a (y, x) position. Huh? Encrypting a string or a character makes sense, but what does it mean to encrypt a position?

check_mirror_position() is unintuitively named, since I would expect a method named check_something to validate rather than build the result.

find_char_in_mirror() makes reasonable sense — it is the only method that works by accepting a parameter and returning its result, with no side-effects! It's not very efficient, though — it potentially scans the entire mirrorField to find the character. Also, since it could be considered a "private" method, it would be conventional to name it _find_char_in_mirror().

What should be done to reorganize the code?

• Rename start() to encrypt().

• Rename private methods to more meaningful names, with a leading underscore.

• Change all instance variables to local variables.

• Change mirrorField from a static variable to an instance variable, and rename it mirror_field to comply with PEP 8.

• Change all methods to return useful values.

```
class MirrorEncrypt(object):
"""
String encryption using a 13x13 mirror field
"""

def __init__(self):
# TODO: read mirror_field from somewhere
self.mirror_field = (…)

def encrypt(self, string):
"""Encrypt the string with the mirror field as the key, returning the
encrypted string."""
encrypted = ''
for char in string:
y, x = self._find_char_in_mirror(char)

# Set direction based on the starting position
direction = ('down' if y == 0 else
'up' if y == 14 else
'left' if x == 14 else
'right')

encrypted += self._follow(y, x, direction)
return encrypted

def _follow(self, y, x, direction):
"""Return the resulting character from tracing the beam from the given
position and direction."""
while True:
if direction == 'left':
x -= 1
direction = self._turn(y, x, direction, 'down', 'up')
if direction == 'right':
x += 1
direction = self._turn(y, x, direction, 'up', 'down')
if direction == 'up':
y -= 1
direction = self._turn(y, x, direction, 'right', 'left')
if direction == 'down':
y += 1
direction = self._turn(y, x, direction, 'left', 'right')
if self.mirror_field[y][x] and self.mirror_field[y][x] not in '\\/':
return self.mirror_field[y][x]

def _turn(self, y, x, direction, direction_slash, direction_backslash):
"""Determine the new direction of the beam."""
if self.mirror_field[y][x] == '/':
return direction_slash
elif self.mirror_field[y][x] == '\\':
return direction_backslash
else:
return direction

def _find_char_in_mirror(self, char):
"""..."""
for y, row in enumerate(self.mirror_field):
for x, col in enumerate(self.mirror_field[y]):
if self.mirror_field[y][x] == char:
return y, x

encrypt = M

StackExchange Code Review Q#131660, answer score: 3