SVG class with boilerplate prologue and epilogue from files I noticed this old class of mine:

class SvgDrawing:

    def __init__(self, print_layout):
        self.print_layout = print_layout

        with open(os.path.join(os.path.dirname(os.path.abspath(__file__)), 'svg_start.txt'), mode='r') as f:
            self.svg_start = f.read()

        with open(os.path.join(os.path.dirname(os.path.abspath(__file__)), 'svg_end.txt'), mode='r') as f:
            self.svg_end = f.read()


And I thought I would refactor it like this to avoid re-reading the same file every time an instance is created.

class SvgDrawing:

    with open(os.path.join(os.path.dirname(os.path.abspath(__file__)), 'svg_start.txt'), mode='r') as f:
        svg_start = f.read()

    with open(os.path.join(os.path.dirname(os.path.abspath(__file__)), 'svg_end.txt'), mode='r') as f:
        svg_end = f.read()

    def __init__(self, print_layout):
        self.print_layout = print_layout


The new code works and improves the performance, but... something is telling me that there should be a better way.

Is it correct to put code that does something more complex than just assigning constant values in the class definition?
Is it correct to put code that does something more complex than just assigning constant values in the class definition?




In general, I try to avoid it. Any code in the class definition will be run as soon as the class definition is executed – that is, as soon as somebody imports your code. So putting complex code in the class definition causes problems for importers:


If the code is slow, you slow down the rest of the program
If the code has side effects, you’re breaking the rule of least surprise – it’s quite unusual for importing a module to have side effects
If the code can throw exceptions (e.g. if you try to read svg_start.txt and it’s not there), same thing. When I import a library, the only exception I expect it might throw would be an ImportError. Throwing an OSError is generally quite unusual.


So I’d try to avoid it if you can. Only run that code when you need it – i.e., when somebody starts making instances of the class.



In this case, I’d think of maybe defining a descriptor, which caches the file contents, and then apply that to the class, like so:

class CachedFile:

    def __init__(self, path):
        self.path = path
        self.contents = None

    def __get__(self, instance, type):
        if self.contents is None:
            self.contents = open(self.path).read()
        return self.contents


class SVGDrawing:

    svg_start = CachedFile(os.path.join(os.path.dirname(os.path.abspath(__file__)), 'svg_start.txt'))
    svg_end = CachedFile(os.path.join(os.path.dirname(os.path.abspath(__file__)), 'svg_end.txt'))

    def __init__(self, *args, **kwargs):
        pass




Note that in this case, the file reads are deferred right up to the point where we need them – much more efficient than doing a read at import time. The flipside is that this code is a bit more complicated – you should decide if the performance gain is worth it.