Skip to main content
2 of 2
Link to the docs
Peilonrayz
  • 43.5k
  • 7
  • 76
  • 155
  1. Rather than os and os.path use pathlib.Path.

    def _save_config_data(content):
        CONFIG.parent.mkdir(parents=True, exist_ok=True)
        with open(CONFIG, 'w') as f:
            json.dump(obj=content, fp=f, indent=2)
    
    
    def _load_config_data():
        file = CONFIG if CONFIG.exists() else DEFAULT_CONFIG
        with open(file, 'r') as f:
            return json.load(f)
    
  2. Your code doesn't follow idiomatic Python. Python uses errors for control flow so returning True/False in set_setting is not idiomatic.

  3. You should make a walk function which you call from both get_setting and set_setting.

  4. Reduce is generally not a good solution. I would say in the specific example of your code, get_setting_old has far superior readability to get_setting.

import os
import json
from functools import reduce

from definitions import CONFIG, DEFAULT_CONFIG

__all__ = [
    "get_setting",
    "set_setting",
]


# Helper functions

def _save_config_data(content):
    CONFIG.parent.mkdir(parents=True, exist_ok=True)
    with open(CONFIG, 'w') as f:
        json.dump(obj=content, fp=f, indent=2)


def _load_config_data():
    file = CONFIG if CONFIG.exists() else DEFAULT_CONFIG
    with open(file, 'r') as f:
        return json.load(f)


def _walk(obj, path):
    for segment in path:
        obj = obj[segment]
    return obj


def get_setting(*args, default=_SENTINEL):
    try:
        return _walk(_CONFIG_DATA, args)
    except LookupError:
        if default is _SENTINEL:
            raise LookupError(f"cannot walk path; {args}") from None
        else:
            return default


def set_setting(value, *args):
    *args, segment = args
    try:
        node = _walk(_CONFIG_DATA, args)
        node[segment] = value
    except LookupError:
        raise LookupError(f"cannot set path; {args}") from None


_SENTINEL = object()
_CONFIG_DATA = _load_config_data()
  1. I think using a class would be better as then you can define __getitem__ and __setitem__ for a potentially nicer interface.
class Walker:
    def __init__(self, data):
        self._data = data
        
    def __getitem__(self, args):
        if not isinstance(args, tuple):
            args = (args,)
        return self.get(*args)

    def __setitem__(self, args, value):
        if not isinstance(args, tuple):
            args = (args,)
        self.set(value, *args)

    def _walk(self, obj, path):
        for segment in path:
            obj = obj[segment]
        return obj

    def get(self, *args, default=_SENTINEL):
        try:
            return _walk(_CONFIG_DATA, args)
        except LookupError:
            if default is _SENTINEL:
                raise LookupError(f"cannot walk path; {args}") from None
            else:
                return default

    def set(self, value, *args):
        *args, segment = args
        try:
            node = _walk(_CONFIG_DATA, args)
            node[segment] = value
        except LookupError:
            raise LookupError(f"cannot set path; {args}") from None
Peilonrayz
  • 43.5k
  • 7
  • 76
  • 155