django-appsettings icon indicating copy to clipboard operation
django-appsettings copied to clipboard

Add an attribute with the default value to Setting

Open ziima opened this issue 7 years ago • 5 comments

Several Setting subclasses override their __init__ just to provide a different default value. We should add an attribute which would replace the need to override the __init__ just in this case.

ziima avatar Sep 19 '18 07:09 ziima

Can you show an example? I'm not sure to understand how you would do this.

pawamoy avatar Sep 19 '18 12:09 pawamoy

I think about something like:

class Settings(object):
    default_default_value = None

    def __init__(self, ..., default=NOT_PROVIDED, ...):

    @property
    def default_value(self):
        if self.default == NOT_PROVIDED:
            return self.default_default_value  # Give or take the callable
        ... # Current implementation

class DictSettings(Settings):
   default_default_value = dict

I'm in a trouble with names, since both default and default_value are already used. So those are definitely temporary.

ziima avatar Sep 19 '18 14:09 ziima

OK I see, thanks!

What about this:

SENTINEL = object()

class Settings(object):
    default = None

    def __init__(self, ..., default=SENTINEL, ...):
        ...
        if default != SENTINEL:
            self.default = default
        # otherwise self.default is already set with the class attribute
        ...

    # default_value property remains unchanged

class DictSettings(Settings):
    default = dict

Though I'm not sure it's good practice.

By the way it could be applied to other arguments if needed.

pawamoy avatar Sep 19 '18 16:09 pawamoy

I don't like the class attribute to be change to instance attribute when passed as argument to __init__. I'm more in favor of having those two separated.

Other arguments might benefit from this approach as well, but I'd rather start with one ;-)

ziima avatar Sep 20 '18 09:09 ziima

OK for separating them! And I honestly can't find a better name than default_default_value. Unless if we use uppercase, like DEFAULT or DEFAULT_VALUE, but it's a bit ugly.

pawamoy avatar Sep 22 '18 12:09 pawamoy