HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Recursively patching the properties of an ORM model and related models

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thepropertiesormrelatedrecursivelymodelandmodelspatching

Problem

The following excerpt is part of a method for recursively patching the properties of an ORM model and related models using a dictionary:

try:
    anhaenge = d['anhaenge']
except KeyError:
    self.logger.debug('anhaenge unchanged')
else:
    try:
        anhang = anhaenge['anhang']
    except KeyError:
        self.logger.debug('anhaenge.anhang unchanged')
    except TypeError:  # anhaenge is probably None
        for record in self.anhang:
            yield (record, PatchMode.DELETE)
    else:
        for record in self.anhang:
            yield (record, PatchMode.DELETE)

        if anhang:
            for anhang_ in anhang:
                with suppress(EmptyDict):
                    for record in Anhang.from_dict(
                            anhang_, immobilie=self):
                        yield (record, PatchMode.CREATE)


I currently violate the don't repeat yourself (DRY) principle, since I do the very same operation within the block handling the TypeError and at the beginning of the inner else block.
How can I resolve this to have the code

for record in self.anhang:
    yield (record, PatchMode.DELETE)


only once?

Clarifying the input:

The inputd is a dictionary derived by json.loads() from a JSON string through a web API.
The method, the above excerpt is extracted from uses this dictionary to modify database records represented by the ORM model this methods belongs to.
Examples of d:

# Delete all subsequent anhang records
d = {'anhaenge': {
    'anhang': None}}

# Change all related anhang records to those represented by the dicts in
# the list, meaning:
# 1) Delete all old records
# 2) Create new ones from the dicts
d = {'anhaenge': {
    'anhang': [
        {'anhangtitel': 'Attachment1'},
        {'anhangtitel': 'Attachment2'}]}}

# All subsequent anhang records are left as they are.
# Subsequent foo records may be altered accordingly
d = {'anhaenge': {
    'foo': 'bar'}}

Solution

If we assume that either for loop can come before or after the other for loop.
Then you should check if anhaenge is not None.
As your except TypeError is to guard against that.
You can then return from the function after you log the error, remove the except TypeError, remove the duplicate for loop,
and finally, put the duplicate for loop after the if, as you wish to perform the loop here on either branch. EG:

try:
    anhaenge = d['anhaenge']
except KeyError:
    self.logger.debug('anhaenge unchanged')
else:
    if anhaenge is not None:
        try:
            anhang = anhaenge['anhang']:
        except KeyError:
            self.logger.debug('anhaenge.anhang unchanged')
            return
        else:
            for anhang_ in anhang:
                with suppress(EmptyDict):
                    for record in Anhang.from_dict(anhang_, immobilie=self):
                        yield record, PatchMode.CREATE
    for record in self.anhang:
        yield (record, PatchMode.DELETE)


But this is not possible if the duplicated for loop has to occur before the other for loop.

Either way, I honestly don't think you're input is right.
Your input is strange, None is normally used to say it's not there, and so dict.get('a', None) is a common occurrence.
However your code performs differently whether it's None or not there.
And so I'd say you should restructure the input so you can use say:

anhang = d.get('anhaenge', {}).get('anhang', None)
if anhang:
    for record in self.anhang:
        yield (record, PatchMode.DELETE)

    for anhang_ in anhang:
        with suppress(EmptyDict):
            for record in Anhang.from_dict(
                    anhang_, immobilie=self):
                yield (record, PatchMode.CREATE)
else:
    self.logger.debug('anhaenge or anhaenge.anhang is unchanged')


If you can't adapt either of the above, then I'd say your code is good for your situation.

Code Snippets

try:
    anhaenge = d['anhaenge']
except KeyError:
    self.logger.debug('anhaenge unchanged')
else:
    if anhaenge is not None:
        try:
            anhang = anhaenge['anhang']:
        except KeyError:
            self.logger.debug('anhaenge.anhang unchanged')
            return
        else:
            for anhang_ in anhang:
                with suppress(EmptyDict):
                    for record in Anhang.from_dict(anhang_, immobilie=self):
                        yield record, PatchMode.CREATE
    for record in self.anhang:
        yield (record, PatchMode.DELETE)
anhang = d.get('anhaenge', {}).get('anhang', None)
if anhang:
    for record in self.anhang:
        yield (record, PatchMode.DELETE)

    for anhang_ in anhang:
        with suppress(EmptyDict):
            for record in Anhang.from_dict(
                    anhang_, immobilie=self):
                yield (record, PatchMode.CREATE)
else:
    self.logger.debug('anhaenge or anhaenge.anhang is unchanged')

Context

StackExchange Code Review Q#146566, answer score: 2

Revisions (0)

No revisions yet.