2
\$\begingroup\$

Context:

A logger records events which contain an area, a level, a message and an option indicating that the source replaces another one.

The logger attempts to send the message through HTTP, and on failure, saves it locally. If HTTP service times out, the logger stops attempting using HTTP and stores the message locally directly.

Concern:

I'm concerned with code duplication at the end of the method. The same call is repeated three times. In my opinion:

  • Creating a function within the method would be an overkill.

  • Creating a separate class to encompass the four elements and pass an instance of this class would be an overkill as well.

  • Moving the call to the end and using return would complicate the logic.

What are my options?

class Logger():
    ...
    def _log(self, area, level, message, replacing=None):
        ...
        if self._timedOut:
            self._logToFile(area, level, message, replacing) # ← This line...
            return

        try:
            self._post(json)
        except urllib.error.HTTPError:
            self._logToFile(area, level, message, replacing) # ← ... is duplicated here...
        except socket.timeout:
            self._logToFile(area, level, message, replacing) # ← ... and here.
            self._timedOut = True
\$\endgroup\$
2
\$\begingroup\$

Method decomposition looks like the way to go here:

def _try_post(self, json):
    """Try posting the message (unless this timed out previously).
    Return True if successful, False otherwise.

    """
    if not self._timedOut:
        try:
            self._post(json)
            return True
        except urllib.error.HTTPError:
            pass
        except socket.timeout:
            self._timedOut = True
    return False
| improve this answer | |
\$\endgroup\$

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