6
\$\begingroup\$

I'm new to Python, and my first serious project was a FreeCell game. The game works, with lots of options to combine moves, save games, load them, etc. But I skimped on some specifics of Python. In particular, reading in options.

import re

vertical = False
doubles = False
autoReshuf = False
savePosition = False
saveOnWin = False
annoyingNudge = False

def readOpts():
    global vertical
    global autoReshuf
    global doubles
    global saveOnWin
    global savePosition
    global annoyingNudge
    infile = "fcopt.txt"
    with open(infile) as f:
        for line in f:
            gotOne = 1
            if line[0] == '#': #ignore comments
                continue
            q=re.sub(r'.*=', '', line.rstrip())
            if "autoReshuf".lower() in line.lower():
                autoReshuf = TOrF(q)
            if "savePosition".lower() in line.lower():
                savePosition = TOrF(q)
            if "saveOnWin".lower() in line.lower():
                saveOnWin = TOrF(q)
            if "vertical".lower() in line.lower():
                vertical = TOrF(q)
            if "doubles".lower() in line.lower():
                doubles = TOrF(q)
            if "annoyingNudge".lower() in line.lower():
                annoyingNudge = TOrF(q)
    if gotOne:
        print "Options file read."
        f.close()
    else:
        print "Failed to read options file."
    return

def TOrF(x):
    if x == "False" or x == "0":
        return False
    return True

readOpts();

The options file I have features lines such as this, with #s to comment lines out:

vertical=1
doubles=0
autoReshuf=1
savePosition=0
saveOnWin=0
annoyingNudge=True

I have a feeling I could simplify this code greatly with ConfigParser, but I'm totally busted at the moment, even with examples. And I don't like throwing all those globals around. How can I write this brute-force code more succinctly and efficiently?

\$\endgroup\$
  • \$\begingroup\$ The fact that savePosition=false would actually set it to True is unintuitive. \$\endgroup\$ – 200_success Dec 13 '16 at 20:35
4
\$\begingroup\$

You have a good idea on what to do. But rather than using global instead use local objects. But even better use a dictionary.

Currently you have something like:

my_global_variable = 'default'

def read_settings():
    global my_global_variable
    my_global_variable = 'something else'

def foo():
    if my_global_variable == 'something else':
        # Do something

However this means that you have to keep everything in your global scope. Rather than this, I'd change to use a dictionary. To do this you can set all the defaults in a single variable, and then over-write them in your read_settings file. Take:

settings = {
    'key': 'default'
}

def read_settings():
    settings['key'] = 'something else'

def foo(SETTINGS):
    if SETTINGS['key'] == 'something else':
        # Do something

This allows you to later move these functions into their own modules, and they will still be usable. And so I'd go on to change the original code to use a dictionary to hold the settings.

I would also change:

  • Remove re and instead use str.split, it makes the code easier to understand.
    If XKDC is your think they even suggest not using them:

    If you're having' Perl problems I feel bad for you, son- I got 99 problems, so I used regular expressions. Now I have 100 problems.

  • Check if the left hand side is actually correct, autoReshufAn'Ting shouldn't pass, and be allocated to autoReshuf.

  • Change your settings to have better keywords, like auto_reshuffle. The use of _ allows you to use only lower-case letters and for your keys to still be readable.
  • Remove the if/else after you read the file. The else is wrong as you were able to read the options file, otherwise there would be an error.
  • Don't use semicolons, there's no need in Python.
  • Follow PEP8.

Without using dictionarys, you can get something like:

vertical = False
doubles = False
auto_reshuffle = False
save_position = False
save_on_win = False
annoying_nudge = False

def read_settings(infile="fcopt.txt"):
    global vertical
    global doubles
    global auto_reshuffle
    global save_position
    global save_on_win
    global annoying_nudge

    with open(infile) as f:
        for line in f:
            # ignore comments
            if line[0] == '#':
                continue
            key, value = line.split("=", 1)
            if key.lower() == "vertical":
                vertical = true_or_false(value)
            if key.lower() == "doubles":
                doubles = true_or_false(value)
            if key.lower() == "auto_reshuffle":
                auto_reshuffle = true_or_false(value)
            if key.lower() == "save_position":
                save_position = true_or_false(value)
            if key.lower() == "save_on_win":
                save_on_win = true_or_false(value)
            if key.lower() == "annoying_nudge":
                annoying_nudge = true_or_false(value)

def true_or_false(x):
    if x == "False" or x == "0":
        return False
    return True

read_settings()

After this you want to use the dictionary as I suggested. With this you can use in to check if key.lower() is in it, and allows you to remove all the duplicate code. After checking if the key is in the dictionary, you want to overwrite the default value in the dictionary, so you can just assign the true_or_false index the dictionary, by indexing with the lowercase key.

This can get something like:

SETTINGS = {
    'vertical': False,
    'doubles': False,
    'auto_reshuffle': False,
    'save_position': False,
    'save_on_win': False,
    'annoying_nudge': False,
}

def read_settings(infile="fcopt.txt"):
    with open(infile) as f:
        for line in f:
            # ignore comments
            if line[0] == '#':
                continue
            key, value = line.split("=", 1)
            key = key.lower()
            if key in SETTINGS:
                SETTINGS[key] = true_or_false(value)

def true_or_false(x):
    if x == "False" or x == "0":
        return False
    return True

read_settings()

And so if you want to add a new value to your settings you can! Just by adding it to the dictionary.


You can also try using json to make a config for you. I personally use it as my default data storage. And with by using sort_keys=True and indent=4, you can make a pretty configuration. Which doesn't support comments. Or alternately you can import a Python file with a dictionary, shown below, or use configparser.

setting.py:

settings = {
    'VerTicaL': True,
}

main.py:

import settings

settings = {k.lower(): v for k, v in settings.settings.iteritems()}
\$\endgroup\$
  • \$\begingroup\$ A very belated thanks for this. I didn't have comment privileges before due to low rep, so I couldn't post, but...I've been able to do pretty much all you've suggested, and I've come back to your response several times. \$\endgroup\$ – aschultz Apr 27 '17 at 17:11
  • 1
    \$\begingroup\$ @aschultz Late is better than never, :) Thank you for your comment, I'm happy I could help! \$\endgroup\$ – Peilonrayz Apr 27 '17 at 17:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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