3
\$\begingroup\$

I have written a simple flashcard program that runs in the terminal:

  • It loads the cards from an ordinary .txt file.

  • It provides a way to add flashcard from within the program to avoid that beginners editing the plain-text mess the formatting up.

  • It asks questions one after the other, because it maximizes learning (no asking of "do you want to continue?")

  • It does not check input for validity, by design, I have a library to check for input validity but I am not using it to allow people to more easily run my code.

  • It deletes a question if you say it correctly 3 times in one session and duplicates it each time you say it wrong. This way you are asked on your weaknesses most often than your strengths.

  • It contains no noise as it clears the screen before asking each question.

  • It should run in a terminal as the code used for cleaning the screen does not work in IDLE (why would you run this in the IDLE anyway? It can not even be made fullscreen.)

  • It is about 50 lines long, also counting blank lines and imports, so it does suffer code bloat.

  • If you give a wrong answer it sleeps more than when you give a correct answer, so you can have time to read the correction.

  • It captures CTRL+C and goes back to asking if you want to add or view flashcards. If you want to close it in one keypress, use CTRL+D

  • It uses one global variable even if global variables should generally be avoided because: practicality beats purity

import random
import time
from collections import defaultdict

FLASHCARD_FILENAME = "flashcards.txt"

correct_dict = defaultdict(int)

def clear_terminal():
    # Credit to: http://stackoverflow.com/questions/2084508/clear-terminal-in-python
    print(chr(27) + "[2J")

def remove_string(filename, string):
    with open(filename) as f:
        content = f.read()
    with open(filename, "w+") as ff:
        ff.write(content.replace(string, '', 1))

def add_flashcard():
    question = input("Enter the question for the new flashcard: ")
    answer = input("Enter the answer for the new flashcard: ")
    with open(FLASHCARD_FILENAME, "a") as f:
        f.write("{} : {}\n".format(question, answer))

def view_flashcard():
    with open(FLASHCARD_FILENAME, "r") as f:
        choice = random.choice(f.readlines())

    clear_terminal()

    question, answer = choice.split(" : ")
    correct = input(question + " ").lower() == answer.replace("\n",'').lower()
    print("\nCorrect!" if correct else "The correct answer was: '{}'".format(answer.replace("\n",'')))

    if not correct:
        with open(FLASHCARD_FILENAME, "a") as f:
            f.write(choice)
    else:
        correct_dict[choice] += 1
        if correct_dict[choice] % 3 == 0:
            remove_string(FLASHCARD_FILENAME, choice)

    time.sleep(2 if not correct else 0.5)

if __name__ == "__main__":
    while True:
        action = [add_flashcard, view_flashcard][int(input("\nEnter 0 to add flashcards, 1 to view them: "))]

        while True:
            try:
                action()
            except KeyboardInterrupt:
                break

Example flashcards.txt file (English irregular verbs):

say : said said
make : made made
go : went gone
take : took taken
come : came come
see : saw seen
know : knew known
get : got got
give : gave given
find : found found
think : thought thought
tell : told told
become : became become
show : showed shown
leave : left left
feel : felt felt
put : put put
bring : brought brought
begin : began begun
keep : kept kept
hold : held held
write : wrote written
stand : stood stood
hear : heard heard
let : let let
mean : meant meant
set : set set
meet : met met
run : ran run
pay : paid paid
sit : sat sat
speak : spoke spoken
lie : lay lain
lead : led led
read : read read
grow : grew grown
lose : lost lost
fall : fell fallen
send : sent sent
build : built built
understand : understood understood
draw : drew drawn
break : broke broken
spend : spent spent
cut : cut cut
rise : rose risen
drive : drove driven
buy : bought bought
wear : wore worn
choose : chose chosen
cut : cut cut
become : became become
hold : held held
lie : lay lain
find : found found
come : came come
draw : drew drawn
cut : cut cut
\$\endgroup\$

2 Answers 2

3
\$\begingroup\$
  • I would use os.system('cls' if os.name == 'nt' else 'clear') rather than ANSI, if you need to support for all terminal emulators. I have had problems with '\b', so I don't have much faith in cmd. (I know the cool kids use PowerShell now... but cmd is still there!)

  • I personally dislike that you are reading the 'file based dictionary'. I would prefer to not ware out my hard-drive. After one day of card revision, and bam, my hard-drive no longer works.
    The price you pay for passing exams...

  • Python prefers to limit the line length to 79, as then things are more readable. For example we don't need to scroll in CR. And this is also why newspapers use columns.

  • I personally would fail silently on the keyboard interrupt to exit the program. Leaving with an error is extremely ugly.

  • If you run out of cards, then your program abruptly stops. I would at least explain something to the user. (This may be part of that extra validation you removed).


I would write a 'constructor' and 'deconstructor' for this program, which would be the only two times you read or write to the file.
To hold the data, I would use the following structure:

{
    'say': [3, 'said said'],
    ...
}

If you didn't know you can use finally to execute something even if the program fails. This is immensely helpful for the deconstructor.
This will look like:

if __name__ == "__main__":
    load_file()
    try:
        main()
    except KeyboardInterrupt:
        print(chr(27) + "[36m goodbye!" + chr(27) + "[0m")
    finally:
        save_file()

Now if you change all the file operations to use the dictionary instead. The state of your program will be easier to understand. And my review would be over.


As someone who supports using the battery's in Python when needed, you can remove defaultdict if you change to a dict, rather than a file. And you can use csv or json to store your data.

Your file looks like a csv with a different delimiter... So you may want to try that out.


If I were to re-write your code: (It's longer, but pretty colours!)

import random
from time import sleep

FLASHCARD_FILENAME = "flashcards.txt"
questions = {}


def clear_terminal():
    print(chr(27) + "[2J")


def add_question(question, answer):
    try:
        questions[question][0] += 3
    except KeyError:
        questions[question] = [3, answer.strip().lower()]


def load_file():
    with open(FLASHCARD_FILENAME) as f:
        for line in f:
            add_question(*line.split(' : '))


def save_file():
    with open(FLASHCARD_FILENAME, 'w') as f:
        for question, info in questions.items():
            output = question + ' : ' + info[1] + '\n'
            for _ in range((info[0] + 2) // 3):
                f.write(output)


def add_flashcard():
    add_question(
        input("Enter the question for the new flashcard: "),
        input("Enter the answer for the new flashcard: ")
    )


def view_flashcard():
    clear_terminal()

    try:
        question = random.choice(list(questions.keys()))
    except IndexError:
        print("We have run out of questions.")
        raise KeyboardInterrupt

    answer = questions[question][1]
    correct = input(question + " ").lower() == answer
    print("{0}[32mCorrect!{0}[0m".format(chr(27))
          if correct else
          "The correct answer was: '{}'".format(answer))

    info = questions[question]
    if not correct:
        info[0] += 3
    else:
        info[0] -= 1
        if not info[0]:
            del questions[question]

    sleep(2 if not correct else 0.5)


def main():
    while True:
        tmp = int(input("\nEnter 0 to add flashcards, 1 to view them: "))
        action = [add_flashcard, view_flashcard][tmp]
        try:
            while True:
                action()
        except KeyboardInterrupt:
            pass

if __name__ == "__main__":
    load_file()
    try:
        main()
    except KeyboardInterrupt:
        print("\n" + chr(27) + "[36m goodbye!" + chr(27) + "[0m")
    finally:
        save_file()
\$\endgroup\$
2
\$\begingroup\$

You've clearly been around, and most of your code does follow the PEP8 guidelines, with some minor exceptions:

  • Use two blank lines between top level functions – This opens up the file and helps separate the functions apart
  • Add docstrings and comments to neccessary parts – Whilst most of it is intuitive, there are some parts that are not as intuitive. I.e. the usages of correct_dict and action list, and non-intuitive usage on how to end program

But all in all, stylewise it looks quite nice.

Code review

I do have some more issues when reviewing your design choices for this utility:

  • No clear way of actually ending it – How do you end it? How many knows that you can use Ctrl-C to issue the keyboard interrupt? And this doesn't work in all environments either. When I run your program from "Spyder-Py2", I have to use the end program button to actually stop it. I would add a stop word, i.e. 'quit' or an empty string to terminate program
  • Little or no instructions – Even though your clear the terminal to avoid seeing previous answers, you could provide a little information. When the screen only says 'see', you don't have much information as to what to do. I see you call it "no noise", but some instructions are always good to have, and since you clear the terminal, there is plenty of space
  • Why correct_dict[choice] % 3 == 0? – This is counter intuitive, how can it be more than 3 in your current logic, and why not simply test on equals 3 (or larger than 3?) Why the modulo operation?
  • Ineffective file handling – Why do you read the entire file for each time you want to review a new flashcard? You are opening, reading, appending and writing the file way too many times. You should rather read it once at startup into a list, and use the list to keep your extras and removals when answering false or correct.
  • Why the destructive file handling? – If you have a successful session, you'll lost the original contents of your flashcard file. Never to be seen again. To me, it would make more sense to leave it intact for secondary runs.
  • Change sleep vs correct/wrong status printing – To me it makes more sense to remove the sleep (which didn't work correctly within Spyder as it didn't output the messages before after the sleep, and then the terminal cleared), and rather state the status when asking the new question.
  • Separate action list from action input – To jam these together makes it hard to read. It is a neat option to use a list for actions, but it doesn't read well. It is better to split these, so that you have a list of actions, and then after doing your input, you choose in this list.
  • Add error handling – All too often we skip error handling, which is a little sad. Especially in such a utility program that this is with file handling it should have error handling if the file doesn't exist, isn't readable or writable, and similar cases. This also applies to if the action is not a legal action.

    And what if the flashcard file is empty, or you complete the set? Neither of these situations are handled in your current code.

  • Allow user to specify flashcard file – If the user could specify the flashcard file as an argument, you would make this utility a lot more useful as you could prepare multiple sets

Code refactored

Here is my version (written originally in Python 2.7, but I think it should run directly in Python 3):

import random
import os
import sys
from collections import defaultdict

# input = raw_input  # Crude Python 2 hack...

FLASHCARD_FILENAME = "flashcards.txt"

CORRECT_ANSWER_COUNT = 3


def clear_terminal():
    """Clear the terminal independent of OS.

    Credit to: http://stackoverflow.com/questions/2084508/clear-terminal-in-python
    """
    os.system('cls' if os.name == 'nt' else 'clear')


def read_flashcard_file(filename):
    """Reads all flashcards from file into a list of (question, answer)."""

    flashcards = []    
    with open(filename) as flashcard_file:
        for line in flashcard_file:
            # Split line on colon into question and answer
            question, answer = line.lower().split(":")

            # Remove white space around each, and append to flashcard list
            flashcards.append((question.strip(), answer.strip()))

    return flashcards


def write_flashcard_file(filename, flashcards):
    """Overwrite filename with new flashcards."""

    with open(filename, "w+") as filename:
        for flashcard in flashcards:
            filename.write('{0} : {1}\n'.format(*flashcard))


def add_flashcards(filename):
    """Add multiple new flashcards to your current flashcard file."""

    flashcards = read_flashcard_file(filename)

    print("Enter 'end' at question prompt to stop adding questions")
    print("Enter 'cancel' at question prompt to abort all additions")

    question = ''
    while question != 'end':
        question = input("\nEnter the question for the new flashcard: ").lower().strip()
        answer = input("Enter the answer for the new flashcard: ").lower().strip()

        if question == 'cancel':
            break
        elif question == 'end':
            continue

        flashcard = (question, answer)

        if flashcard in flashcards:
            print("This flashcard already exists in file. Not added")
        else:
            flashcards.append(flashcard)

    else:
        # Only called on normal ending of while loop
        write_flashcard_file(filename, flashcards)


def remove_flashcards(filename):
    """Remove multiple flashcards from your current flashcard file."""

    # Read flashcards from file
    flashcards = read_flashcard_file(filename)


    print("Enter 'end' at question prompt to stop removing questions")
    print("Enter 'cancel' at question prompt to abort all removals")

    remove_question = ''
    while remove_question != 'end':
        remove_question = input("\nEnter the question to remove: ").lower().strip()

        if remove_question == 'cancel':
            break
        elif remove_question == 'end':
            continue

        question_indexes = [ idx for idx, (question, answer) in enumerate(flashcards)
                                 if question == remove_question]

        if not question_indexes:
            print("Couldn't find question: '{}'".format(remove_question))

        else:
            for idx in question_indexes:
                del flashcards[idx]

    else:
        # Only called on normal ending of while loop
        write_flashcard_file(filename, flashcards)


def review_flashcards(filename):
    """Review all flashcards, repeating if wrong, and clearing if correct."""

    flashcards = read_flashcard_file(filename)

    # Will hold how many times a flashcard has been answered correctly
    correct_answer = defaultdict(int)

    previous_answer = None

    while True:
        clear_terminal()

        print("Use 'end' as answer to stop reviewing.")

        if previous_answer:
            print("Your previous answer was: {}".format(previous_answer))

#        print("Correct_answer: {}".format(correct_answer))
#        print("Flashcards: {}".format(flashcards))

        # Select a random flashcard, and store both as flashcard and split version
        if flashcards:
            question, answer = flashcard = random.choice(flashcards)
        else:
            print("Congratulations!!! You've completed the entire set")
            break


        new_answer = input("\n {} : ".format(question)).lower().strip()

        if new_answer == 'end':
            break

        correct = new_answer == answer

        previous_answer = "Correct!" if correct else "Wrong, the correct answer was: '{}'".format(answer)

        if not correct:
            # Add the question to list again, to increase testing on it
            flashcards.append(flashcard)

        else:
            # If answered correctly a given number of times, remove from list
            correct_answer[flashcard] += 1

            if correct_answer[flashcard] >= CORRECT_ANSWER_COUNT:
                # Remove all instances of that flashcard from list                
                for remove_idx in reversed([idx for idx, element in enumerate(flashcards)
                                       if element == flashcard]):
                 del flashcards[remove_idx]



# Defined here instead of top, to be able to refer to the functions directly
ACTIONS = {
    "review" : (review_flashcards, "Review flashcards"),
    "add" : (add_flashcards, "Add one or more flashcard to set"), 
    "remove" : (remove_flashcards, "Remove one or more flashcards from set")
    }


def main(filename):
    """Main loop asking and handling: add, remove and review of flashcards."""

    while True:
        print("\nYour actions are:")
        print("    'end' : To end program")
        for idx, (action_choice, (_, action_text)) in enumerate(list(ACTIONS.items())):
            print ("    '{}' : {}".format(action_choice, action_text))


        action_choice = input("Action: ").lower().strip()

        if action_choice == 'end':
            break

        elif action_choice in ACTIONS:
            ACTIONS[action_choice][0](filename)

        else:
            print("Illegal option, please choice again")


if __name__ == "__main__":
    # Enter some magic to choose filename from arguments...
    print('args: {}'.format(sys.argv))
    if len(sys.argv) == 2 and sys.argv[1] != '':
        filename = sys.argv[1]
    else:
        filename = FLASHCARD_FILENAME

    main(filename)

Some comments to my own code:

  • I haven't added error handling related to file operations
  • I've used the while...else structure instead of exception handling to provide two alternative ways out of add_flashcards and remove_flashcards. The else of a loop is entered if the loops terminates normally, that is if break is not called. This is used to give the user an option to cancel or abort the edit operation on flashcard file
  • When removing a question from the list, I remove it from all places in the list. This calls for searching through the list and removing them in reversed order. If not done in reversed order, the list of indexes will be wrong as the list shifts when you delete items.
  • I've opted to keep the "end" action out of the ACTIONS list, to emphasize that this is not an ordinary action. It could be added to the list, but I still think I would have kept that as a separate choice to show how to terminate out of the loop
  • I've added descriptions to the ACTIONS list using a tuple. This could probably be done in a nicer manner, but as it stands it does allow for easily adding new functions which automatically will be included in the action menu. This also keeps the menu and action items closely in sync
  • The filename selection from arguments is rudimentary, and you should most likely better use argparse or docopts modules to do this. The code presented is more a proof of concept, rather than a good implementation.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.