Skip to main content
added 563 characters in body
Source Link

And if you want to, EAFP can also help reduce the overhead of the hasattr test:

def load_template(template_name):
    try:
        environment = load_template.environment
    except AttributeError:
        cwd = os.path.dirname(os.path.abspath(__file__))
        madlib_dir = os.path.join(cwd, '..', 'templates', 'madlib')
        loader = jinja2.FileSystemLoader(searchpath=madlib_dir)
        load_template.environment = environment = jinja2.Environment(loader=loader)
   return environment.get_template(template_name)

And if you want to, EAFP can also help reduce the overhead of the hasattr test:

def load_template(template_name):
    try:
        environment = load_template.environment
    except AttributeError:
        cwd = os.path.dirname(os.path.abspath(__file__))
        madlib_dir = os.path.join(cwd, '..', 'templates', 'madlib')
        loader = jinja2.FileSystemLoader(searchpath=madlib_dir)
        load_template.environment = environment = jinja2.Environment(loader=loader)
   return environment.get_template(template_name)
Source Link

For starter, I'd rewrite get_env a bit and introduce intermediate variables for readability:

def get_env(self):
    if not self._environment:
        cwd = os.path.dirname(os.path.abspath(__file__))
        madlib_dir = os.path.join(cwd, '..', 'templates', 'madlib')
        loader = jinja2.FileSystemLoader(searchpath=madlib_dir)
        self._environment = jinja2.Environment(loader=loader)
    return self._environment

Next I would cleanup your @property usage; since the raw decorator already defines the getter property, you can get rid of the first method:

class Template(object):
    def __init__(self):
        self._environment = None

    def __call__(self, temp_name):
        return self.get_env.get_template(temp_name)

    @property
    def get_env(self):
        if not self._environment:
            cwd = os.path.dirname(os.path.abspath(__file__))
            madlib_dir = os.path.join(cwd, '..', 'templates', 'madlib')
            loader = jinja2.FileSystemLoader(searchpath=madlib_dir)
            self._environment = jinja2.Environment(loader=loader)
        return self._environment

Lastly, since the initialization of _environment is meant to be done (and thus checked) only once, why not take advantage of attribute access that already offer us this kind of flexibility:

class Template(object):
    def __call__(self, temp_name):
        return self.environment.get_template(temp_name)

    def __getattr__(self, attribute):
        if attribute == 'environment':
            cwd = os.path.dirname(os.path.abspath(__file__))
            madlib_dir = os.path.join(cwd, '..', 'templates', 'madlib')
            loader = jinja2.FileSystemLoader(searchpath=madlib_dir)
            self.environment = env = jinja2.Environment(loader=loader)
            return env
        return super(Template, self).__getattr__(attribute)

This way, __getattr__ is only called the first time self.environment is accessed and never again. But... Hold on... The only usefull method of this class is __call__... Better write a function instead; this will bring the check back at each call but will make the intent clearer:

def load_template(template_name):
    if not hasattr(load_template, 'environment'):
        cwd = os.path.dirname(os.path.abspath(__file__))
        madlib_dir = os.path.join(cwd, '..', 'templates', 'madlib')
        loader = jinja2.FileSystemLoader(searchpath=madlib_dir)
        load_template.environment = jinja2.Environment(loader=loader)
   return load_template.environment.get_template(template_name)

So now, no need to pass an object around everywhere, just call the function when you need to.