Initializing instance variables from json file

Question

I have a class whose instance variables should be initialized from a file ('settings.json'). That file does not always exist or is sometimes not filled with useful values, so I have to check for that first. I am now wondering what the best practice would be, I currently see two options but I am open to better ones.

1. Check if the file exists and contains useful information in __init__() and then create and fill the instance variables:

   class Settings:
         def __init__(self):
             if file.exists() and contains_useful_information():
                 self.dir_path = "path/to/dir"
                 ...
             else:
                 # ask for user input
                 pass
   
   

2. First create placeholders and fill them later after checking the file.

   class Settings:
         def __init__(self):
             self.dir_path = ""
             self.check_file()
   
         def check_file(self):
             # check if file exists and contains useful values, otherwise ask user for input
             self.dir_path = "path/to/dir"
   
   

I don't really like putting this much logic into __init__() but I don't see a better solution.

Background: I'm using PyQt5 to build a GUI. I currently have two classes, one that deals with the actual graphical interface and one that deals with settings (e.g. directory paths). Since I want some of these settings to be available to the user without reentering them every time, they are saved in a settings file.

This is the first time I'm using OOP in Python or rather in general, so I'm not really sure. I found https://stackoverflow.com/questions/5875902/how-to-initialize-a-class-with-data-from-a-python-file and this https://codereview.stackexchange.com/questions/104841/read-file-into-list-when-class-is-instantiated-in-python-2-7, but they are both not really solving my problem, since the first does not contain logic and the second is something I can solve without using a class.

Edit: I now found these questions: Pattern for creating an instance of a class from a UI form and Instantiate a class from a config file. Where should the parse function go?. While these are nearer to my question, I still feel they do not completely answer my question, as they both do the parsing somewhere else. I could also do this like suggested in the second question in the main function before calling the class. Would that be a better option?

Answer 1

To keep your Settings clean and tidy, let them do one thing: represent Settings.

What you should do is: provide sane defaults.

What you could do is: accepting a dictionary with custom settings.

class Settings:
    def __init__(self, options=None):
        defaults = {"port": "8080"}
        if options is not None:
            for k, v in defaults.items():
                value = options.get(k, v)
                defaults[k] = value

        self.__dict__.update(**defaults)

If you feel fancy, you could add a classmethod doing the loading of a json file:

class Settings:
    @classmethod
    def from_json(cls, filename):
        with open(filename) as f:
            options = json.load(f)
        return cls(options)

    def __init__(self, options=None):
        defaults = {"port": "8080"}
        if options is not None:
            for k, v in defaults.items():
                value = options.get(k, v)
                defaults[k] = value

        self.__dict__.update(**defaults)

The point is:

1. You separate the behaviour of your actual settings from the loading process. The caller could provide alternative options independently from the source - as long as they are provided as a dictionary. The class is fully usable without ever calling from_json().

Loading settings from a file in a classmethod gives the user of your class a convenient way of building an instance of your settings object.

2. Dealing with non-existing files is not your problem. It is the problem of the caller of the classmethod.

Answer 2

In general, you should avoid putting too much logic into the constructor method because it can quickly become difficult to maintain.

Maybe, if you want to avoid putting too much logic directly into your class, you can give a try to the recipe 9.21 suggested in the Python Cookbook. The solution I am suggesting falls in the category of Metaprogramming techniques. As the aforementioned book says:

In a nutshell, metaprogramming is about creating functions and classes whose main goal is to manipulate code (e.g., modifying, generating, or wrapping existing code). The main features for this include decorators, class decorators, and metaclasses.

So, instead of having a method inside a class that performs some kind of input validation, you can write an external function that you call within your class.

Take a look at this question on Stackoverflow for getting the code used by the above recipe and to get a full and working example. In that case, I wanted to perform input validation for write-once read-many attributes.

Obviously, you have to modify a little bit the code of the recipe to fit your needs, but I believe it should work for you case as well.

For instance, the logic you put into the check_file method could be moved inside the external function check_file in the following way:

import os

def check_file(file_path_variable_name):
    storage_name = '_' + file_path_variable_name
        
    @property
    def prop(self):
        return getattr(self, storage_name)
    
    @prop.setter
    def prop(self, value):
        if not os.path.isfile(value): # for instance check if file is found
            raise FileNotFoundError('File not found.')
        setattr(self, storage_name, value)
    return prop
    

class Settings:
    file_path = check_file('file_path')
    
    def __init__(self, file_path):
        self.file_path = file_path

Now your class is very clean, easy to understand and easy to maintain. Moreover, you can reuse the function check_file in other classes/modules in your current project or in other projects.

For your convenience, I also paste here the code that you can find in the Stackoverflow link above:

class ReadOnlyError(Exception):
    """Attempt made to assign a new value to something that can't be changed."""


# Based on recipe in book "Python Cookbook 3rd Edition" - section 9.21 -
# titled "Avoiding Repetitive Property Methods".
def readonly_typed_property(name, expected_type):
    storage_name = '_' + name

    @property
    def prop(self):
        return getattr(self, storage_name)

    @prop.setter
    def prop(self, value):
        if hasattr(self, storage_name):
            raise ReadOnlyError('{!r} is read-only!'.format(name))
        if not isinstance(value, expected_type):
            raise TypeError('{!r} must be a {!r}'.format(name, expected_type.__name__))
        setattr(self, storage_name, value)

    return prop


class Point:
    x = readonly_typed_property('x', int)
    y = readonly_typed_property('y', int)

    def __init__(self, x, y):
        self.x = x
        self.y = y