Using a class for reading config from files

Question

The goal of my current piece of work is to read from a global config file all the settings and then from the local file only those settings that a user has chosen.

Firstly I wrote a function but then I needed a class. It's my first working class in Python, and I ask if you can kindly note its defects. Although it works, I think it can for sure be written more correctly and concisely.

Here's all the beginning:

# files to be read from
cmd_parameter = "config_1.cfg"
cmd_parameter_global = "global.cfg"
if len(sys.argv) == 2:
 cmd_parameter = sys.argv[1]
else:
 print("Error: there should be one argument after program name - config file name")
 exit(-1)
config_file = []
config_file_global = []
# reading user's config file
try:
 with open(cmd_parameter, 'r') as config_file_name:
 config_file = config_file_name.readlines()
except FileNotFoundError:
 print("File '", sys.argv[1], "' not exists or not available")
 exit(-1)
# reading global config file
try:
 with open(cmd_parameter_global, 'r') as config_file_global_name:
 config_file_global = config_file_global_name.readlines()
except FileNotFoundError:
 print("Global configuration file '", cmd_parameter_global, "' not exists or not available")
 exit(-1)
# parsing config file, reading values from it to list and matching parameters with a given argument
# parameters in config file should start with <conf_key_split1>, value should be separated from parameter by <conf_key_split2>
config_param_user = []
config_value_user = []
config_param_global = []
config_value_global = []
config = ""
conf_key_split1 = '.'
conf_key_split2 = '='
conf_key_split3 = ','
# reading user's config file
for line in config_file:
 if line.startswith(conf_key_split1):
 line = line.rstrip().split(conf_key_split1)
 line = line[1]
 line = line.rstrip().split(conf_key_split2)
 config_param_user.append(line[0])
 config_value_user.append(line[1])
# reading global config file
for line in config_file_global:
 if line.startswith(conf_key_split1):
 line = line.rstrip().split(conf_key_split1)
 line = line[1]
 line = line.rstrip().split(conf_key_split2)
 config_param_global.append(line[0])
 config_value_global.append(line[1])

And here is the class itself:

# class for reading config from user's and global config file the list of params names list and the list of values
class configFromFile():
 def config_from_file_userfile(self, param):
 index = config_param_user.index(param)
 if config_value_user[index]:
 config_value = config_value_user[index]
 else:
 print('The value of the parameter "', param, '" was not found in', cmd_parameter)
 exit(-1)
 return config_value
 def config_from_file_global(self, param):
 index = config_param_global.index(param)
 if config_value_global[index]:
 config_value = config_value_global[index]
 else:
 print('The value of the parameter "', param, '" was not found in', cmd_parameter_global)
 exit(-1)
 return config_value
 def main(self, param):
 if param in config_param_global:
 config = self.config_from_file_global(param)
 elif param in config_param_user:
 config = self.config_from_file_userfile(param)
 else:
 print('The parameter "', param, '" was not found in', cmd_parameter, 'and in', cmd_parameter_global)
 exit(-1)
 return config

Usage:

names = configFromFile().main('names')

• Roman Susi: You can also use ChainMap (docs.python.org/3/library/collections.html#chainmap-objects ) objects to represent your configuration information with arbitrary number of chained configuration files. Documentation do have examples.
• jonrsharpe: Please see meta.codereview.stackexchange.com/q/1763/32391

Answer 1

Firstly, you state:

I wrote a function but then I needed a class

Looking at the class you wrote, I would say you were incorrect there. The class has no __init__, holds no state, and only uses self to access other methods on the class. There is absolutely no reason for it to be a class.

---

Duplication is the programmer's arch enemy - every time you have the same thing written out more than once is an opportunity to reduce the amount of code you write, test and maintain. For example:

# reading user's config file
try:
 with open(cmd_parameter, 'r') as config_file_name:
 config_file = config_file_name.readlines()
except FileNotFoundError:
 print("File '", sys.argv[1], "' not exists or not available")
 exit(-1)
# reading global config file
try:
 with open(cmd_parameter_global, 'r') as config_file_global_name:
 config_file_global = config_file_global_name.readlines()
except FileNotFoundError:
 print("Global configuration file '", cmd_parameter_global, "' not exists or not available")
 exit(-1)

Note that these do more-or-less exactly the same thing, and should therefore be refactored to something like:

def extract_file(filename, message):
 try:
 with open(filename) as file_:
 return file_.readlines()
 except FileNotFoundError:
 print(message.format(filename))
 exit(-1)

and then called like:

config_file = extract_file(cmd_parameter,
 "File '{}' not exists or not available")

You should apply this throughout your code - you have basically written the same program twice, with minor adjustments, for the two config files, which makes no sense.

---

The dict is a key data structure in Python, and would make all of your code vastly simpler. You could create a dictionary of the global configuration options, then update it with the local values. Lookups by name would then be O(1), rather than O(n), and you could get rid of all the indexing.

---

In terms of general structure, you have lots of code running at the top level of the script. This is generally a bad idea, and will give you problems if you later want to import this functionality elsewhere. You should define an entry point (usually, but not necessarily, called main) that takes the file locations as parameters and call it something like:

if __name__ == "__main__":
 if len(sys.argv) == 2:
 main('global.cfg', sys.argv[1])
 else:
 print("Error: there should be one argument after program name - config file name")
 exit(-1)

• nrp: Thanks a lot for answering. About code at the top level - I'm currently writing functions, and what you've written is very handy for me now. I'll probably leave the class because it is comfortable for me to have structure like that when I can just add more methods inside if there would be, say, more config files.
• jonrsharpe: @nonrandom_passer but those methods are all duplicates. If you adopt the dictionary version, all you need is one function to parse a specified file to a config dictionary, then dict.update in the correct order. You could also use an existing file format, e.g. JSON, rather than define your own.
• nrp: First of all, thanks again for your remarks. I used the dict and updated the main question, pls take a look. And I also would appreciate if you'll comment on 2 more things. First is an entry point that you've mentioned (sorry, I didn't quite got it - did you mean to put all the main body of a script into a main method?
• nrp: And the second is that I've tried to put all imports inside try-except blocks that I have also in the beginning to one function but did not succeed. Can you please comment? def import_module(module_name, custom_module_name, message): try: import module_name as custom_module_name except ImportError: print(message.format(module_name)) import_module("random", "rnd", "Importing module '{}' did not succeed")
• jonrsharpe: Putting the whole script inside a main is one way to do it, but it would be much better to split into lots of short functions, then have a main that just calls the appropriate functions at the appropriate times.