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\$

closed as off-topic by Graipher, Toby Speight, Stephen Rauch, yuri, Sᴀᴍ Onᴇᴌᴀ Aug 20 '18 at 14:59

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." – Graipher, Toby Speight, Stephen Rauch, yuri, Sᴀᴍ Onᴇᴌᴀ
If this question can be reworded to fit the rules in the help center, please edit the question.

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
\$\endgroup\$

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