7
\$\begingroup\$

Task:

The task requires creating a class (say, ConfigDict) which inherits from the dict class.

It is supposed to save its keys and values in a file. If a file does not exist, the instance of ConfigDict should be able to create it.

Examples:

For instance, the following code:

fh=open("config.txt","w")
fh.write("key1 = 5\n")
fh.write("key2 = 3\n")
fh.close()
cd=ConfigDict("config.txt")
cd.items()

would generate an output dict_items([('key1', '5'), ('key2', '3')]).

If a file does not exists, it should be possible for the instance to create it and save its data inside it.

dd=ConfigDict("test.txt")
dd["keyA"] = 'testA'
dd["keyB"] = 'testB'
dd.items()

The above lines would produce dict_items([('keyA', 'testA'), ('keyB', 'testB')]) (and a print message).

My attempt:

I will greatly appreciate any comments.

class ConfigDict(dict):
    def __init__(self,file):
        self.file = file

        try: #check if file exists
            with open(self.file,"r+") as fh:
                for line in fh:
                    key, item = line.replace(" ", "").rstrip().split("=")
                    dict.__setitem__(self,key,item)

        except: 
            print("creating "+file)
            fh=open(self.file,"w")
            fh.close()


    def __setitem__(self,key,item):
        dict.__setitem__(self,key,item) #to avoid infinite loop
        lines = open(self.file).read().splitlines()

        for index, line in enumerate(lines):
            if line.replace(" ", "").rstrip().split("=")[0] == key:
               lines[index] = str(key)+" = "+str(item)
            else:
                lines.append(str(key)+" = "+str(item))

        open(self.file,'w').write('\n'.join(lines))

Corrected version

import os

class ConfigDict(dict):
    def __init__(self,file):
        self.file = file
        if os.path.isfile(self.file):
            with open(self.file,"r") as fh:
                for line in fh:
                    key, item = line.replace(" ", "").rstrip().split("=")
                    dict.__setitem__(self,key,item)

    def __setitem__(self,key,item):
        dict.__setitem__(self,key,item)            
        with open(self.file, 'w') as save_file:
            for key, value in self.items():
                save_file.write("{} = {}\n".format(key, value))
\$\endgroup\$
6
\$\begingroup\$

bugs:

  1. Your implementation of __setitem__() does not to add a line if the key does not exist in the file yet.

other issues:

  1. There is no need to create the file if it does not exist in __init__(), as open(..., 'w') in __setattr__() will create it for you.

  2. It is gennerally not a good idea to print from a data class (i.e. print("creating "+file)) as this makes the class unuseable for programs that use stdout.

  3. Iterating over a file in python automatically splits on lines so

    lines = open(self.file).read().splitlines()
    for index, line in enumerate(lines):
    

    could be written as

    for index, line in enumerate(open(self.file)):
    

    (like you did in __init__())

  4. If you are going to completly rewrite the file on every __setitem__(), there is no point in reading the file first, as this takes more time and is harder to read. here is an example __setitem__():

    def __setitem__(self,key,item):
        dict.__setitem__(self,key,item) #to avoid infinite loop
    
        with open(self.file, 'w') as save_file:
            for key, value in self.items():
                save_file.write("{} = {}\n".format(key, value))
    
\$\endgroup\$
  • \$\begingroup\$ Thanks, the code will be much simpler. Regarding point 1, while I'm aware that open(...,'w') creates a file, if the try statement seemed to be a reasonable choice. If a file exists, one can set the keys and values. If it does not exist, create it and do nothing else. \$\endgroup\$ – Gregory Rut Jun 23 '18 at 21:04
  • 1
    \$\begingroup\$ My point is that the file will be created anyways in __setattr__() so there is no reason to create it in __init__(). I have edited my answer to hopefully reflect this better. \$\endgroup\$ – Peter Jun 23 '18 at 21:17
  • \$\begingroup\$ Ok, I think i get it:-). \$\endgroup\$ – Gregory Rut Jun 23 '18 at 21:29
  • 2
    \$\begingroup\$ Oh and please, close the file in __setitem__. Preferably using a with statement. \$\endgroup\$ – Mathias Ettinger Jun 24 '18 at 19:13
  • 3
    \$\begingroup\$ @GregoryRut instead of writing the whole file at each __setitem__, just write the last entry, it will take less time. Duplicate entries are taken care off just fine when reading the file. Special care has to be taken when deleting entries, hence the proposal to rewrite the file on __delitem__. \$\endgroup\$ – Mathias Ettinger Jun 24 '18 at 19:52

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.