5

I only have OOP programming experience in Java and have just started working on a project in Python and I'm starting to realize Python makes me skeptical about a few things that are intuitive to me. So I have a few questions about OOP in Python.

Scenario: I'm writing a program that will send emails. For an email, the to, from, text and subject fields will be required and other fields like cc and bcc will be optional. Also, there will be a bunch of classes that will implement the core mail functionality, so they will derive from a base class (Mailer).

Following is my incomplete code snippet:

class Mailer(object):
    __metaclass__ == abc.ABCMeta

    def __init__(self,key):
        self.key = key

    @abc.abstractmethod
    def send_email(self, mailReq):
        pass


class MailGunMailer(Mailer):
    def __init__(self,key):
        super(MailGunMailer, self).__init__(key)

    def send_email(self, mailReq):
        from = mailReq.from
        to = mailReq.to
        subject= mailReq.subject
        text = mailReq.text
        options = getattr(mailReq,'options',None)
        if(options != None):
            if MailRequestOptions.BCC in options:
                #use this property
                pass
            if MailRequestOptions.CC in options:
                #use this property
                pass



class MailRequest():
    def __init__(self,from,to,subject,text):
        self.from = from
        self.to = to
        self.subject = subject
        self.text = text

    def set_options(self,options):
        self.options = options

class MailRequestOptions():
    BCC = "bcc"
    CC = "cc"

Questions:

  1. The send_mail method can take multiple parameters (from, to, subject, text, cc, bcc, etc.) and only four of them are really required in my app. Since the number of parameters for the method is too high, I decided to create a wrapper object called MailRequest, which will have the four necessary parameters as properties, and all other parameters may be defined in the options dictionary. Problem is, here, just looking at the code there's no way to say that options is. Is it a dict or a list? Also, looking at the send_email method, there's also no way to tell what mailReq is. Is this bad programming practice? Should I be doing something else? Coming from the Java world, it makes me very uncomfortable to write code where you can't tell what the parameters are by just looking at the code. I got to know about annotations in Python, but I don't want to use them since they're only supported in later versions.

  2. Since the options dict should be used to specify many other properties (cc and bcc are just two of them), I've created a new class called MailRequestOptions with all of the options that may be specified inside the options dict as MailRequestOptionss static strings. Is this bad practice as well, or is there a better way to do this? This is not really Python specific, I know.

jonrsharpe
  • 115,751
  • 26
  • 228
  • 437
GrowinMan
  • 4,891
  • 12
  • 41
  • 58
  • 3
    This is not about Python vs Java, this is about static type inference vs dynamic type inference: http://en.wikipedia.org/wiki/Type_inference – Reut Sharabani Apr 01 '15 at 09:38

3 Answers3

5

Python is a "duck-typed" language; if it walks like a duck, and quacks like a duck, it's a duck! Or, in implementation terms, if the object passed as mailReq has the from, to, subject and text attributes, it doesn't really matter whether or not it's a MailRequest.

If you want to document the interface (which is certainly a good idea), it is conventional to use docstrings to do so. I like the Google style, which can be used with sphinx-napoleon to auto-generate human-readable documentation, but others are available.

def send_email(self, mailReq):
    """Send an email.

    Args:
      mailReq (MailRequest): the configuration for the email.

    """
    ...

As to your second question; wrapping lots of arguments into a container object is a pretty common pattern. In Python, you have the option of making things a bit simpler using "**kwargs magic" (see e.g. What does ** (double star) and * (star) do for parameters?):

def send_email(self, from_, to, subject, text, **config):
    ...
    bcc = config.get('bcc', [])  # option from config or a default
    ...

(note that from is a keyword in Python, so can't be the name of a parameter).

This has the advantage of being reasonably self-documenting - there are four required parameters, plus some arbitrary additional keyword configuration options (which, again, would normally be documented in the docstring).

Community
  • 1
  • 1
jonrsharpe
  • 115,751
  • 26
  • 228
  • 437
  • Nice explanation. However, being the arguments of an email so defined (from, to, subject, text, bbc and cc), I would not go for `**kwargs` magic, but specifying them in the method signature. It is less confusing for newbies and more self-documented. – bgusach Apr 01 '15 at 10:07
  • @ikaros45 I think the OP's point is that `cc` and `bcc` are just examples, that there are quite a few additional options. Too many parameters is a definite code smell, and encapsulating them is a common fix (see the link in my answer, http://stackoverflow.com/a/816517/3001761, etc. - `pylint` also has a warning for this). – jonrsharpe Apr 01 '15 at 10:10
  • That is true, too many arguments stink and should be logically grouped. On the other hand, explicit arguments are more self documentating and enhance testability. I don't love too much the usage of kwargs for these cases because you have to write very detailed docstrings, or you are forcing the user of your code to read your source. In this case I would define a class to group the args (as the OP did) and defining in its `__init__` the required arguments. In this example, the instances should have a `.cc` attribute which would return an empty list if it was not passed on construction. – bgusach Apr 01 '15 at 10:24
  • @ikaros45 I think this is a reasonable approach; do write up a dissenting opinion if you disagree. – jonrsharpe Apr 01 '15 at 10:33
  • It is indeed. I think it was just interesting to point that the flexibility of `**kwargs` is powerful, but it worsens readability. – bgusach Apr 01 '15 at 10:37
  • jonrsharpe & @ikaros45 my other question was about the MailRequestOptions. I was planning to define that class to just contain class-level strings so that the user can know which options can be set. Does that look acceptable, or do you think there is a better alternative? And you're right jon, cc and bcc are just examples. there can be more. – GrowinMan Apr 01 '15 at 19:51
  • Also, @jonrsharpe, given that cc & bcc are just two examples and there could be 15 other such optional parameters, do you still think **kwargs is a reasonable option here? From what I gather, when you use **kwargs, you can't just pass a dictionary, but you have to specify all the arguments with keys when calling the function. We wouldn't want the use to specify 15 kwargs in the function call, right? – GrowinMan Apr 01 '15 at 20:00
  • @GrowinMan as pointed out in Alex's answer, you can encapsulate the options in a dictionary and unpack it to the method call. Having a custom object to hold them is also fine, though. – jonrsharpe Apr 01 '15 at 20:14
  • @jonrsharpe, finally any comment on the MailRequestOptions class? – GrowinMan Apr 01 '15 at 20:18
  • @GrowinMan haven't I already made several? Is there something that's unclear? – jonrsharpe Apr 01 '15 at 20:45
  • @jonrsharpe I'm not sure if using the MailRequestOptions class just to specify the different option the user may be able to set seems like an acceptable idea or not. I don't think you've made any comment about that thing specifically. – GrowinMan Apr 01 '15 at 21:01
  • @GrowinMan *"wrapping lots of arguments into a container object is a pretty common pattern"*. also, ikaros45's whole answer covers this pattern. – jonrsharpe Apr 01 '15 at 21:11
  • @jonrsharpe I thought that applied to `MailRequest` not `MailRequestOptions` – GrowinMan Apr 01 '15 at 22:08
4
  1. In Python there is no need to specifically create another object. You can use a dictionary if you want to wrap the mail request: mailReq = {'from': 'johnsmith@british.com', 'to': '....', ...}

  2. You should try to use *args and **kwargs for the method. It can make the options much simpler: def send_mail(from, to, subject, text, **kwargs), then other options can be retrieved using e.g. kwargs['bcc']. I believe this would be more Pythonic.

jonrsharpe
  • 115,751
  • 26
  • 228
  • 437
Alex
  • 370
  • 1
  • 6
  • 11
  • given that cc & bcc are just two examples and there could be 15 other such optional parameters, do you still think **kwargs is a reasonable option here? From what I gather, when you use **kwargs, you can't just pass a dictionary, but you have to specify all the arguments with keys when calling the function. We wouldn't want the use to specify 15 kwargs in the function call, right? – GrowinMan Apr 01 '15 at 20:03
  • Bear in mind that **kwargs magic does not avoid argument explosion. If your function needs 15 arguments and you are not grouping them, you do need to pass 15 arguments. They are just not visible in the function signature. – bgusach Apr 19 '15 at 00:18
1

While jonrsharpe has provided an excellent solution, I think it is worth mentioning my approach.

As already mentioned, in dynamic languages you don't care about types, just about the interface the object has (the so called "duck"). Then, your MailRequest object is an excellent way of grouping the arguments that logically belong together. It does not, however implement everything it should. This would be my approach:

class MailRequest(object):
    def __init__(self, from_, to, subject, text, bcc=None, cc=None):
        # I am asuming a good default for bbc is an empty list. If none
        # is fine, just remove the None checks.
        # Dont get confused about this, it just avoids a pitfall with
        # mutable default arguments. There are other techniques however

        if bcc is None:
            bcc = []

        if cc is None:
            cc = []

        self.from_ = from_
        self.to = to
        self.subject = subject
        self.text = text
        self.bcc = bcc
        self.cc = cc

    # No options needed

And then the send_email function would look like following:

def send_email(self, mailReq):
    """
    :type mailReq: MailRequest

    """
    from_ = mailReq.from_
    to = mailReq.to
    subject= mailReq.subject
    text = mailReq.text
    bcc = mailReq.bcc
    cc = mailReq.cc

Notice that you just document the mailReq argument, pointing that any object passed should provide the MailRequest interface (at least partially). This way, you delegate the documentation of arguments to the MailRequest class.

I prefer this approach over the **kwargs magic because arguments are explicitly passed at some point to a rigid signature, which serves somehow as documentation. The drawback is the verbosity.

EDIT

If you fear arguments explosion in the MailRequest "constructor", the solution is to do the same one level deeper: group again. For instance, you may want to group the options into its own class:

class MailRequestOpts(object):

    def __init__(self, bbc=None, cc=None, colour=None, lights='blue', blink=True):
        # ...
        self.bbc = bbc
        self.cc = cc 
        self.colour = colour
        # etc...

And then the MailRequestClass would look like this:

class MailRequest(object):
    def __init__(self, from_, to, subject, text, options=None):
        """
        :type options: MailRequestOpts

        """
        if options is None:
            options = MailRequestOpts()

        # ...
        self.options = options

At the end if you need 50 arguments for a process, you can't avoid passing 'em all to one or several distributed functions at some point. How many times you group them is up to you and where you find the balance.

bgusach
  • 14,527
  • 14
  • 51
  • 68
  • Just a note: you called "BCC" "BBC" several times. – Nic Apr 01 '15 at 11:05
  • I always loved that news channel. Corrected, thanks :) – bgusach Apr 01 '15 at 11:38
  • I thought about this, and this looks fine for just two extra fields - cc & bcc. But what would you suggest if I had many more optional fields? Having the constructor take 15 arguments didn't sound like a good idea. – GrowinMan Apr 01 '15 at 19:53
  • I'd love to get @ronrsharpe 's opinion on this – GrowinMan Apr 01 '15 at 19:53
  • The only solution to avoid argument explosion is to group them in objects again. I updated my answer with an example. This is however a non-python specific issue, in Java you would have the same problem. – bgusach Apr 01 '15 at 20:13