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?

200_success
145k22 gold badges190 silver badges478 bronze badges
asked Dec 13, 2016 at 17:56
\$\endgroup\$
1
  • \$\begingroup\$ The fact that savePosition=false would actually set it to True is unintuitive. \$\endgroup\$ Commented Dec 13, 2016 at 20:35

1 Answer 1

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()}
answered Dec 13, 2016 at 19:54
\$\endgroup\$
2
  • \$\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\$ Commented Apr 27, 2017 at 17:11
  • 1
    \$\begingroup\$ @aschultz Late is better than never, :) Thank you for your comment, I'm happy I could help! \$\endgroup\$ Commented Apr 27, 2017 at 17:41

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.