Using a class for reading config from files

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 , value should be separated from parameter by
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_globa

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)

StackExchange Code Review Q#75418, answer score: 5