1

I am designing a class whose object is instaniated with a user idn which creates a pdf with that user specific details. It should log something depending on success or failure which can be determined only by caller as the success or failure depends on if it was not just created but dispatched which is done by the caller.

However two problems I can see in this design pattern is that:

It does look a bit odd that caller has to log it from outside. What if the init itself fails?

The way I am thinking of logging as a method if this class is probably wrong? But it doesn't seem so since it shares the state, i.e the user. Any recommendations on this design?

Library Code

class create_pdf():
    """
    Class
    """
    def __init__(self, user):
        """
        """
        user
        db_calls_to_get_full_user_details(user)


    def fetch_pdf_dms(self):
        """
        Fetch from DMS
        """
        logic

    def fetch_pdf_engine(self):
        """
        Fetch from a PDF Generator
        """
        # logic that fetches the blob


    def log(self, error = None):
        """
        """
        # If there was no failure and it was a fetch_pdf_engine
        # event then I store the blob into dms for the first time.
        # If the event failed I capture the exception. Finally I 
        #  log the event in the database.

    def fetch_pdf(self):

        pdf = fetch_pdf_dms
        if pdf:
            return pdf
        return fetch_pdf_engine # which also stores to dms first time!


Consumer Code:

method = REQUEST['method']
for each user in users:
    try:
        object = create_pdf(user)
        object.do_something()
        dispatch(method)
        object.log()
    except:
        object.log(error=exception)
0

2 Answers 2

1

Your pdf creating class should not log, in fact, it should not have any knowledge about your log. It should create that pdf and report any issues to its creator/caller. The creator/caller should then decide what to do with it (write to the console, write to a log file, try again, ignore, whatever). Your pdf creator is a problem domain class and it should have a single responsibility with a minimum of dependencies.

1
  • Yeah that is the correct design.
    – Nishant
    Commented Apr 2, 2016 at 11:32
1

Logs are used to track down what happened, usually because something went horribly wrong.

Therefore, much like exceptions, logging should occur very close to the relevant code. I don't want to have to go digging through a mile long call stack to find the code that goes with an error message. Logging should happen the moment you know that something should be logged.

When it comes to logging exceptions, the best way (using the above criteria) is to catch, log, and rethrow exceptions. Full disclosure: I am not fluent in python, but I'm fairly sure this is how you rethrow.

Library Code

class create_pdf():
    """
    Class
    """
    def __init__(self, user):
        """
        """
        try:
          user
          db_calls_to_get_full_user_details(user)
        except:
          # log
          raise

    def fetch_pdf_dms(self):
        """
        try:
          Fetch from DMS
          """
          logic
        except
          #log
          raise

    def fetch_pdf_engine(self):
        """
        try:
          Fetch from a PDF Generator
          """
          # logic that fetches the blob
        except
          #log
          raise


    def log(self, error = None):
        """
        """
        # If there was no failure and it was a fetch_pdf_engine
        # event then I store the blob into dms for the first time.
        # I further the event as failure depending on failure.

    def fetch_pdf(self):
        try:   
          pdf = fetch_pdf_dms
          if pdf:
              return pdf
          return fetch_pdf_engine # which also stores to dms first time!
        except:
          #log
          raise


Consumer Code:

method = REQUEST['method']
for each user in users:
    try:
        object = create_pdf(user)
        object.do_something()
        dispatch(method)
        object.log()
    except:
       #If the consumer code can deal with the exception, handle them here.
       #If not, eliminate the try-catch and let them bubble up the call stack.
2
  • Ah thats not my exact use case, I just wanted to log an "event" related to what I do. The execption thing is something different, but thanks for pointing it out I will add it.
    – Nishant
    Commented Mar 29, 2016 at 14:57
  • 1
    @Nishant That's fine as well. The overall point is to put logging in right when you know what you need to log. The only exception would be if it's in a time-critical point in your code, and the logging takes too much time.
    – CHendrix
    Commented Mar 29, 2016 at 15:08

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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