HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Simple anagram game with names of colors

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
simplewithnamescolorsanagramgame

Problem

This is a very simple anagram game that plays only with names of colors.

I tried to focus on code design. I also wanted it to support further updates. But, my init seems smelly and you will notice it, especially in the main loop.

I was wondering about my naming and commenting/docstrings. I need strong criticism here, without any mercy.

This is the module I've created with the only class:

```
class AnagramGame:

def __init__(self, words=None, rword=None, inp=None,
anagram=None, is_corr_word=False):
self._words = None
self._rword = rword # Stands for random word
self._anagram = anagram
self._inp = inp # User input
self._is_corr_word = is_corr_word # Stands for correct word bool

def get_words(self):
return self._words

def set_words(self, filename):
in_file = open(filename, 'r')
self._words = [in_line.rstrip('\n')
for in_line in in_file.readlines()]
in_file.close()

def del_words(self):
del self._words

words = property(get_words, set_words, del_words)

def get_rword(self):
return self._rword

def set_rword(self, sequence):
import random
self._rword = random.choice(sequence)

def del_rword(self):
del self._rword

rword = property(get_rword, set_rword, del_rword)

def get_anagram(self):
return self._anagram

def set_anagram(self, sequence):
import random
self._anagram = ''.join(random.sample(sequence, len(sequence)))

def del_anagram(self):
del self._anagram

anagram = property(get_anagram, set_anagram, del_anagram)

def get_is_corr_word(self):
return self._is_corr_word

def set_is_corr_word(self, boolean):
self._is_corr_word = boolean

def del_is_corr_word(self):
del self._is_corr_word

is_corr_word = property(get_is_corr_word, set_is_corr_word,
del_is_corr_wor

Solution

In your __init__() function, you have defined words as a parameter but you are assigning None to _words

class AnagramGame:

def __init__(self, words=None, rword=None, inp=None,
             anagram=None, is_corr_word=False):
    self._words = None
    # ...


You may either remove this parameter or assign the value to _words

Instead of using an infinite loop while True:, you should have created a bool object. Something like this:

checker = False
while not checker:
    # ...


I see the purpose of while True: is to make the user play the game endlessly. It would have been great to have an exit criteria too. Else, you could have skipped the infinite loop.

You don't have to re-initialize the object and reload the file on each loop run. You may initialize them before the loop, load the files and let the game run on its own.

To reset the values, you may create another function that would only reset the relevant data-members and continue the game further.

As you have already mentioned, please have proper comments and doc-strings in the code. It is always better to add them during development. Else mostly they get ignored!

Code Snippets

def __init__(self, words=None, rword=None, inp=None,
             anagram=None, is_corr_word=False):
    self._words = None
    # ...
checker = False
while not checker:
    # ...

Context

StackExchange Code Review Q#30417, answer score: 4

Revisions (0)

No revisions yet.