papi-web-org / papi-web

Papi-web
7 stars 3 forks source link

Should we use the `ConfigReader` mapping protocol? #4

Closed Amaras closed 9 months ago

Amaras commented 1 year ago

In Python, the usual way to handle errors is "EAFP" (It's easier to ask for forgiveness than permission). This is especially true for interfacing with the operating system, etc.

The point is that the current version of the code uses a lot of code like this:

section = 'XXX'
if not self.has_section(section):
    # log no section error
else:
    key = 'YYY'
    if not self.has_option(section, key):
        # log no key error
    else:
        var = self.get(section, key)
        # do something with var

This could probably be changed to the following pattern:

try:
    section = self['XXX'] # config_reader['XXX']
    try:
        var = section['YYY']
        # do something with var
    except (KeyError, TypeError): # TypeError can happen if 'XXX' is a key and we expected a section
        # log no key error
except KeyError:
    #log no section error

I don't feel comfortable trying to understand the legacy API that is currently used, so this might be worthwhile (we should not get this to the main branch before equivalence of behaviour is established first, of course)

pascalaubry commented 12 months ago

Yes we should, waiting for your PR to complete ;-)

Amaras commented 11 months ago

Mostly done now with #6

Amaras commented 11 months ago

I can probably be closed if #7 is merged in. I can separate out the mapping protocol changes from that PR if the rest is not wanted.