trjhtr

To train my Python skills I have made a vocabulary trainer which runs on a terminal:

import random
import os
import sys

FILE_PATH = "./words.txt"

class Entry:
 def __init__(self, french, english):
 self.french = french
 self.english = english

entries = 

if os.path.isfile(FILE_PATH):
 words = open(FILE_PATH, "r")

 for line in words:
 words = line.split(",")
 entries.append(Entry(words[0].strip(), words[1].strip()))


def insert():
 while True:
 french = raw_input("French word: ")

 if french == "#":
 return 

 english = raw_input("English word: ")

 if english == "#":
 return 

 entries.append(Entry(french, english))
 backup_wordpairs()

def query():
 total = 0
 right = 0
 wrong = 0

 while True:
 i = random.randint(0, len(entries) - 1)
 french = raw_input("French translation of " + entries[i].english + ": ")

 # TODO: Add a statistics which is displayed before leaving (x of y correct. Equal z percent).
 if french == "#":
 percentage = (right * 100) / total
 print("You answered " + str(right) + " question out of " + str(total) + " correct.")
 print("Percentage: " + str(percentage) + " %")
 return 

 total = total + 1

 if french.strip() == entries[i].french.strip():
 print("Correct.")
 right = right + 1
 else:
 print("Wrong!")
 wrong = wrong + 1

def printall(): 
 if len(entries) == 0:
 print("No words stored.")
 return

 for i in range(len(entries)):
 print(entries[i].french + " : " + entries[i].english)

def backup_wordpairs():
 woerter = open(FILE_PATH, 'w')

 for wort_paar in entries:
 woerter.write(wort_paar.french + "," + wort_paar.english + "n")

 woerter.close()

def reset_list():
 global entries

 entries = 

 if os.path.isfile(FILE_PATH):
 os.remove(FILE_PATH)

while True:
 command = raw_input("Please enter a command: ")

 if command == "add":
 insert()
 elif command == "test":
 query()
 elif command == "list":
 printall()
 elif command == "end":
 break
 elif command == "reset":
 reset_list()
 else:
 print("No known command.")

print(" ------- Vocabulary becomes terminated. ---------- ")
sys.exit()

My GitHub repository (including a README-file with user-instructions and a screenshot).

The application runs fine but I would appreciate feedback from more experienced Python-developers. What would you have done differently and why?

Currently your code is all over the place. You have global variables (which should be named in ALL_CAPS according to Python's official style-guide, PEP8), functions which modify these globals, then some actual code in between the functions and at the end some code that actually executes the code, also in the global namespace (preventing you from importing any of the functions defined here in another script).

In addition I would also change:

1. Use random.choice(entries) instead of random.randint(0, len(entries)).


2. Make Entry a collections.namedtuple (which allows using tuple unpacking).


3. Since you say you are using Python 3.x, use the new f-strings introduced in Python 3.6.


4. Where this is not possible, use str.format instead of building the strings using addition and manual calls to str.


5. Make main take the file path (defaulting to the global constant) so it is more extendable.


6. Making sure to close all opened files again using with ... as.


7. Use if __name__ = "__main__" guard to allow importing.


8. Call strip on the manually entered words, removing the need to do that every time you compare them later.


9. Remove the wrong variable, it is unused (and can be easily calculated as total - right).


10. 
    raw_input is not called that anymore in Python 3. It is now called input (and the input from Python 2 is gone).

import random
import os
import sys
from collections import namedtuple

FILE_PATH = "./words.txt"
Entry = namedtuple("Entry", ["french", "english"])


def insert(entries):
 while True:
 french = input("French word: ").strip()
 if french == "#":
 return
 english = input("English word: ").strip()
 if english == "#":
 return
 entries.append(Entry(french, english))


def query(entries):
 total, right = 0, 0
 while True:
 word_pair = random.choice(entries)
 french = input(f"French translation of word_pair.english:")

 if french == "#":
 percentage = (right * 100) / total
 print(f"You answered right question out of total correct.")
 print(f"Percentage: percentage %")
 return
 total += 1
 if french.strip() == word_pair.french:
 print("Correct.")
 right += 1
 else:
 print("Wrong!")


def printall(entries):
 if len(entries) == 0:
 print("No words stored.")
 return
 for word_pair in entries:
 print(" : ".format(*word_pair))


def backup_wordpairs(entries, file_path):
 with open(file_path, 'w') as words:
 for word_pair in entries:
 words.write(",n".format(*word_pair))


def main(file_path=FILE_PATH):
 if os.path.isfile(file_path):
 with open(file_path) as words:
 entries = [Entry(*map(str.strip, line.split(",")))
 for line in words]
 else:
 print("File does not exist")
 sys.exit()

 while True:
 command = input("Please enter a command: ")

 if command == "add":
 insert(entries)
 backup_wordpairs(entries, file_path)
 elif command == "test":
 query(entries)
 elif command == "list":
 printall(entries)
 elif command == "end":
 break
 elif command == "reset":
 entries.clear()
 if os.path.isfile(file_path):
 os.remove(file_path)
 else:
 print("No known command.")

 print(" ------- Vocabulary becomes terminated. ---------- ")
 sys.exit()

if __name_ == "__main__":
 main()

As you can see now, almost all functions take entries as the first argument and modify it somehow. This should tell you that you could easily make this a class.

Code Organization

For a program of this size, free-floating code should be eliminated. Interleaving free-floating code between class and function definitions is particularly messy:

class Entry:
 ...

entries = 

if os.path.isfile(FILE_PATH):
 ...

def insert():
 ....

The same goes for your while True loop: it should be packaged inside a main() function.

Global variables (namely entries) should be eliminated.

For this program, I would recommend making your program object-oriented, because so many of your functions deal with entries. Specifically, I would split the code into two layers: a database layer and a UI layer. The database (the Vocabulary class in my example below) deals with parsing and writing the file, as well as manipulating vocabulary entries. The UI (the Trainer class) implements the commands (add, test, list, reset, end). The function names should ideally match the corresponding command names.

Properly structured programs should not need to call sys.exit() to exit normally. Execution just naturally ends.

Please don't mix languages when naming variables (e.g. woerter and wort_paar). If the code is mostly in English, then just stick to English names.

Design

The design of your code is too rigid and limited: it only does English-to-French translations. Those two languages are hard-coded into the strings and identifiers. Why not handle French-to-English and more? For very little additional effort, you could easily generalize the program:

• The words.txt file is in CSV format. It could have any number of columns, and does not need to be limited to two languages. The file format could be self-documenting, with the first row containing the language names. (To read and write CSV files, don't split() and concatenate the content yourself; use the csv module.)


• The vocabulary trainer should be given a source language and target language as parameters.

Implementation details

Files should always be open()ed in the context of a with block. That way, the file will automatically be closed when the block exits, even if it exits with an exception.

In the quiz loop, there is no need to keep track of the number of wrong answers, since it can be deduced from total - right. Instead of a while True: loop, I would use for total in itertools.count():.

Concatenating strings is tedious:

print("You answered " + str(right) + " question out of " + str(total) + " correct.")

There are many ways to compose strings in Python. One of the better ways, which is compatible with Python 2 and Python 3, is str.format().

Suggested solution

import csv
from itertools import count
import os
import random
from sys import stdout

class Vocabulary:
 def __init__(self, filename, languages):
 self.filename = filename
 if os.path.isfile(filename):
 with open(filename) as f:
 reader = csv.DictReader(f)
 self.entries = list(reader)
 self.languages = reader.fieldnames
 else:
 self.entries = 
 self.languages = languages

 def clear(self):
 self.entries = 

 def add(self, *entries):
 self.entries.extend(*entries)

 def save(self):
 with open(self.filename, 'w') as f:
 writer = csv.DictWriter(f, self.languages)
 writer.writeheader()
 for entry in self.entries:
 writer.writerow(entry)

 def random_entry(self):
 return random.choice(self.entries)


class Trainer:
 COMMANDS = ['add', 'test', 'list', 'reset']

 def __init__(self, vocabulary, source_language, target_language):
 self.vocabulary = vocabulary
 self.source_language = source_language
 self.target_language = target_language

 def add(self):
 def prompt_entries():
 while True:
 entry = 
 for language in self.vocabulary.languages:
 entry[language] = raw_input(' word: '.format(language))
 if entry[language] == '#':
 return
 yield entry
 self.vocabulary.add(prompt_entries())
 self.vocabulary.save()

 def test(self):
 right = 0
 for total in count():
 entry = self.vocabulary.random_entry()
 q, a = entry[self.source_language], entry[self.target_language]
 ans = raw_input(' translation of : '.format(self.target_language, q))
 if ans == '#':
 break
 elif ans == a:
 print('Correct.')
 right += 1
 else:
 print('Wrong!')
 print('You answered questions out of correctly.'.format(right, total))
 print('Percentage: %'.format(100 * right / total))

 def list(self):
 if not self.vocabulary.entries:
 print("No words stored.")
 else:
 writer = csv.DictWriter(stdout, self.vocabulary.languages, delimiter=':')
 for entry in self.vocabulary.entries:
 writer.writerow(entry)

 def reset(self):
 self.vocabulary.clear()
 self.vocabulary.save()


def main():
 t = Trainer(
 Vocabulary('words.txt', ['English', 'French']),
 source_language='English',
 target_language='French'
 )
 choices = ', '.join(t.COMMANDS + ['end'])
 while True:
 command = raw_input('Please enter a command (): '.format(choices))
 if command == 'end':
 break
 elif command not in t.COMMANDS:
 print('Unknown command.')
 else:
 getattr(t, command)()
 print(' ------- Vocabulary quiz terminated. ---------- ')

if __name__ == '__main__':
 main()