Vigenère decoder/encoder in Python (using matrix)

Question

I made an encoder/decoder for the Vigenère cipher. This one is using a table, not the remainder technique. Please let me know what you think can be done to improve performance, etc.

import string
import itertools


class Vigenere:
    """
    A class for vigenere encoding and decoding.

    Doesn't preserve spaces, UPPERCASE letters.
    Default alphabet is lowercase a-z.
    """

    def __init__(self):
        """Initialize default values."""
        self.__reset()

    def __reset(self):
        # Reset all attributes
        self.alphabet = ""
        self.key = ""
        self.__looped_key = ""
        self.text = ""
        self.__dec_str = ""
        self.__enc_str = ""
        self.__matrix = []

    def __create_matrix(self):
        # create a vigenere matrix
        tmp = 2 * self.alphabet
        self.__matrix.append(list(self.alphabet))
        for i in range(len(self.alphabet) - 1):
            self.__matrix.append(list(tmp.split(tmp[i], 1)[1][:len(self.alphabet)]))

    def __loop_key(self):
        # loop the key and slice so it's length matches text's
        looped = ''.join(list(itertools.repeat(self.key, int(len(self.text) / len(self.key)) + 1)))[:len(self.text)]
        self.__looped_key = looped

    def __set_key(self, key):
        # remove spaces, lower, loop and set the key
        self.key = key.replace(' ', '').lower()
        self.__loop_key()

    def __set_text(self, text):
        # remove spaces, lower and set text
        self.text = text.replace(' ', '').lower()

    def __set_alphabet(self, alphabet):
        # remove spaces and lower
        alphabet = alphabet.replace(' ', '').lower()
        self.__check_alphabet(alphabet)  # check alphabet validity
        self.alphabet = alphabet
        self.__check_chars()  # check key and text compability with alphabet
        self.__create_matrix()  # create a vigenere table

    def __check_alphabet(self, alphabet):
        # check alphabet for duplicates
        if len(alphabet) != len(set(alphabet)):
            raise ValueError('alphabet contains duplicate characters')

    def __check_chars(self):
        # check key and text compability with alphabet
        t_key = ''.join(list(filter(lambda x: x not in self.alphabet, self.key)))
        t_text = ''.join(list(filter(lambda x: x not in self.alphabet, self.text)))
        if t_text != '':
            raise ValueError('text includes characters not in alphabet')
        if t_key != '':
            raise ValueError('key includes characters not in alphabet')

    def decode(self, text, key, alphabet=string.ascii_lowercase):
        """Decode text with key using given alphabet(default a-z)."""
        # reset values
        self.__reset()
        # set attributes
        self.__set_text(text)
        self.__set_key(key)
        self.__set_alphabet(alphabet)
        # iterate through the looped key and decode string
        for i in range(len(self.__looped_key)):
            lst = self.__matrix[self.alphabet.index(self.__looped_key[i])]
            dex = lst.index(self.text[i])
            self.__dec_str += self.__matrix[0][dex]
        return self.__dec_str

    def encode(self, text, key, alphabet=string.ascii_lowercase):
        """Encode text with key using given alphabet(default a-z)."""
        # reset values
        self.__reset()
        # set attributes
        self.__set_text(text)
        self.__set_key(key)
        self.__set_alphabet(alphabet)
        # iterate through the text and encode it
        for i in range(len(self.text)):
            lst = self.__matrix[self.alphabet.index(self.text[i])]
            dex = self.__matrix[0].index(self.__looped_key[i])
            self.__enc_str += lst[dex]
        return self.__enc_str


vigenere = Vigenere()
encode = vigenere.encode
decode = vigenere.decode

if __name__ == '__main__':
    # test
    assert encode('d CO dE', 'KE y') == 'ngmni'
    assert decode('NG mN i', ' ke Y') == 'dcode'
    assert decode('1132xn5m dze HN j5rn9v4Mmzzx qpc7s', 'K ey', 'abcdEF ghijklM Nopqr sT uvwxyz123456789') == 'qwertyuiopasdfghjklzxcvbnm1234'
    assert encode('qW ertYU  iopasdfGhJk LzxcvbNM1234', 'KeY  ', 'abcdEF ghijklM Nopqr sT uvwxyz123456789') == '1132xn5mdzehnj5rn9v4mmzzxqpc7s'

Serge Ballesta · Accepted Answer · 2018-08-28 16:09:05Z

If the question is about both the Python implementation and the used algorithms, it would be nice to say more of the specifications of your class.

After some tests of your code, I can say that you have built a class that can crypt a string using a vigenere cipher and a key. In both the text and the key, the spaces (only spaces, not end of line characters...) are ignored, and upper case characters are converted to lower cases. What remains shall then be alphabets unless you declare a special string of accepted chars. All the data structures are computed for each encoding and each decoding, and you build a looped key by repeating the initial key to make it the length of the text. It would be more memory efficient to loop over the key without storing it in memory. And this design will not be able to encipher a stream of a priori unknown length, what a vigenere cipher can normally do. So I would try to simply iterate the key instead of pre-building a looped key.

Another improvement point is the use of index method from a string. It is perfectly correct but is a linear search in O(n/2) if n is the length of the text. Dictionnaries would allow much more efficient searches by using hash functions.

Finally, a point that I do not really like in your design (nothing more than my opinion here), is that it makes sense to build a single instance of the Vigenere class and use aliases to directly call its public methods. For the same reason, you call the __reset method (already called in __init__) in the public methods, which is equivelent to re-create a new object for each call. IMHO, it means that the instance should have been more specialized, for example with the key and alphabet:

vigenere = Vigenere(key, alphabet = string.ascii_lowercase)
vigenere.encode(string)
vigenere.decode(string)

BTW, that would allow for stream enciphering and deciphering...

Now for the style.

you have doc strings for the class itself and all public methods: good
you have a if __name__ == main: construct: good
you have factorized the __reset method between __init__, encode and decode: good on a style point, but not on a design point
you (almost) consistently use __ private members. Unsure if you really want that or simply single underscore names: the difference is that 2 _ names are mangled with the class name, which you do not really use here.

Thanks for your review. I totally agree with O(n) inefficiency, the point is, I really wanted to mimic manual process of encoding. As for pre-looping the key, I also agree. I will change that. Along with that I will implement spacing preservation, which will work with stream perfectly. For the __reset, I defined that method because I also didn't think about a stream, I made it for given strings so the two calls of encode/decode methods would have nothing in common, so the matrix, looped key and such things had to be reset. The "double privacy" of the methods is useless, I agree. Thanks :) — Nick
– Nick, Commented Aug 28, 2018 at 17:36

Stack Exchange Network

Vigenère decoder/encoder in Python (using matrix)

1 Answer 1

You must log in to answer this question.

Hot Network Questions

Vigenère decoder/encoder in Python (using matrix)

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions