3

I'm loading a config file. I'm considering the following ways of doing it. Version 1:

config = json.load('myconfig.json')
config_elm = config.get('some_config_elm')
if config_elm is not None:
    # use config_elm

... vs version 2:

config = json.load('myconfig.json')
if 'some_config_elm' in config and config['some_config_elm'] is not None:
    config_elm = config['some_config_elm']
    # use config_elm

Valid json files are as follows.

With data set to something:

{
    'a_config_elm': 13,
    'some_config_elm': [42, 43]
}

... set to empty list:

{
    'a_config_elm': 13,
    'some_config_elm': []
}

... without data:

{
    'a_config_elm': 13
}

... with data null'ed out:

{
    'a_config_elm': 13,
    'some_config_elm': null
}

Case 2 and 3 should do the same.

Version 1 seems to be more efficient, but version 2 seems somewhat more clear. Problem with 1 is that an empty list would not pass the if statement, but maybe should. Any thoughts?

  • 2
    The crucial question for me would be what you do if it isn't there. If there's a default value you can continue with, config.get(key, default) makes a lot of sense. If you don't do anything if it's not there, if key in config: would be clearer. If it should fail if it's missing, just config[key]. It's all about communicating your intent. Also you need to decide what if the user explicitly provides null, and what if they provide a non-null but false-y value (if config.get(key) is not None would be better for that reason). But it can't really be answered generally and depends on opinion. – jonrsharpe May 24 '17 at 10:10
  • Exactly. I want it to be false when set to null and non-existing. I'll update so that both do the same. – André Christoffer Andersen May 24 '17 at 10:12
  • @jonrsharpe Both version 1 and 2 should now produce the same output, i.e., I've explicitly handled the None/null case allowing for empty list etc. – André Christoffer Andersen May 24 '17 at 10:15
  • @AndréChristofferAndersen "version 2 seems somewhat more clear" Not for me but to each their own. – Goyo May 24 '17 at 22:23
  • @Goyo Really? To me version 2 reads like English and it is very clear/explicit what it checks for, while version 1 you just have to know that the single check in the if statement covers two eventualities, i.e., "json element doesn't exists" and "json element set to null". But yeah, I guess to each their own. – André Christoffer Andersen May 27 '17 at 16:51
3

I would suggest having a default configuration, with all of your required configuration values in at their default settings, and when you load a configuration file overwriting the settings that are in that file.

Then you don't need to be concerned about possibility of missing entries, they will all be there even if they are missing from the loaded configuration.

I suspect that tools such as mercurial adopt an approach of:

  1. Create the default configuration object
  2. Override with any site wide settings from that configuration file
  3. Further override with any repository specific settings from the current repository
  4. Further override with any command line options given

Of course with this approach any configuration save strategy needs to include a scope of setting for any that you are saving but it is worth the effort as you can add a new configuration setting without having to worry about upgrading existing configuration files, (they just will not include the new setting so will get the defaults).

For implementing this something like:

def LoadSettings(self, filepath):
    """ Add the settings from a current file to this configuration object """
    new_settings_dict = json.load()
    assert isinstance(new_settings_dict, dict), "Error: Config must be a dict"
    for k, v in new_settings_dict.items():
         if hasattr(self, k):
             self[k] = v
         else:
             print("Unknown Configuration Item", k)
0

Given that your settings are set in a dict and your config comes as a dict, what you usually want is doing an update on your settings.

Update the dictionary with the key/value pairs from other, overwriting existing keys.

settings = {
    'a_config_elm': 13,
    'some_config_elm': [42, 43]
}

config = { 'a_config_elm': 14}
settings.update(config)

settings

{'a_config_elm': 14, 'some_config_elm': [42, 43]}

That should fit your needs.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.