75

I'm new to Python and need some advice implementing the scenario below.

I have two classes for managing domains at two different registrars. Both have the same interface, e.g.

class RegistrarA(Object):
    def __init__(self, domain):
        self.domain = domain

    def lookup(self):
        ...

    def register(self, info):
        ...

and

class RegistrarB(object):
    def __init__(self, domain):
        self.domain = domain

    def lookup(self):
        ...

    def register(self, info):
        ...

I would like to create a Domain class that, given a domain name, loads the correct registrar class based on the extension, e.g.

com = Domain('test.com') #load RegistrarA
com.lookup()

biz = Domain('test.biz') #load RegistrarB
biz.lookup()

I know this can be accomplished using a factory function (see below), but is this the best way of doing it or is there a better way using OOP features?

def factory(domain):
  if ...:
    return RegistrarA(domain)
  else:
    return RegistrarB(domain)
smith
  • 371
  • 2
  • 5
  • 14

9 Answers9

87

I think using a function is fine.

The more interesting question is how do you determine which registrar to load? One option is to have an abstract base Registrar class which concrete implementations subclass, then iterate over its __subclasses__() calling an is_registrar_for() class method:

class Registrar(object):
  def __init__(self, domain):
    self.domain = domain

class RegistrarA(Registrar):
  @classmethod
  def is_registrar_for(cls, domain):
    return domain == 'foo.com'

class RegistrarB(Registrar):
  @classmethod
  def is_registrar_for(cls, domain):
    return domain == 'bar.com'


def Domain(domain):
  for cls in Registrar.__subclasses__():
    if cls.is_registrar_for(domain):
      return cls(domain)
  raise ValueError


print Domain('foo.com')
print Domain('bar.com')

This will let you transparently add new Registrars and delegate the decision of which domains each supports, to them.

Alec Thomas
  • 19,639
  • 4
  • 30
  • 24
  • 2
    Hi @Alec. In this particular case, are the decorators (@classmethod) in the classes necessary? If yes, what role do they play in that context? – Morlock Feb 07 '12 at 00:25
  • 1
    Not _strictly_ necessary, no, but without them you'd have to instantiate each Registrar class. The general pattern is what's useful, the @classmethod is just an implementation detail. – Alec Thomas Feb 08 '12 at 12:31
  • 5
    @AlecThomas, `@staticmethod` may have been slightly better to use in this context, I think – dmytro Aug 10 '12 at 08:51
  • 18
    This approach is unsafe unless the concrete subclasses' `is_registrar_for()` are mutually exclusive, and _will remain so in the future_. The order of values returned by `__subclasses__()` is arbitrary. And this order, in general, matters. As a result, if something in the code (perhaps as minor as the order of class definitions) changes, you may end up with a different outcome. The cost of such bugs, IMO, is enormous, and far outweighs the benefits of this approach. I would instead go with the approach the OP used, where a single function contains the entire logic of subclass selection. – max Oct 21 '12 at 02:36
  • 8
    If you do have mutually exclusive tests, or for other reasons feel this approach is safe, be aware that `__subclasses__` returns only immediate subclasses; so multi-level inheritance would require a small tweak to be processed correctly. – max Oct 21 '12 at 02:39
  • 11
    Also be aware that `__subclasses__` **only** works for live objects. If a class is not yet imported it doesn't appear in the results (as it doesn't 'exist'). – siebz0r Dec 04 '13 at 08:28
  • 1
    If each class it defined in its own file, then obviously derived classes import the base class. This means that factory may not be defined in same file as the base class or there will be circular imports - consequently factory function must be defined in its own file. (function rather than @staticmethod as base class method) – Ron Kalian Feb 08 '18 at 13:10
  • 1
    The function capitalisation of `Domain` makes this answer a bit confusing. – naught101 Jan 24 '19 at 00:11
  • @max: valid point, but one function is fine when there are not many cases and is new cases are not expected often. I would argue that when the number of cases is growing some kind of resolution / uniqueness check is needed anyway. It can be done with this approach too. Just do not call return as you find one hit. And, of course, to check upon multi-level inheritance is needed. – serge.v Sep 25 '21 at 12:17
  • And yet another caveat: __subclasses__ contain only weak reference. It is not very probable that derived class type will be garbage collected, but still. So it means that derived classes to be used should be imported and stored in some kind of storage, which is a rather natural thing to do anyway. – serge.v Sep 25 '21 at 12:17
  • @Ron Kalian: not sure to which approach you refer: using __subclasses__ (like in this answer) or plainly go over list (like OP did). If to the second, you are right. But for the first one I have just done it. Defined some derived classes in base class file and some in their own files and it works. With standalone factory function in the base class file, staticmethod and with classmethod. So, there is no circular imports problem. Base class does not import derived classes modules. It (base class type) just gets updated when derived type is created. – serge.v Sep 25 '21 at 12:44
24

Assuming you need separate classes for different registrars (though it's not obvious in your example) your solution looks okay, though RegistrarA and RegistrarB probably share functionality and could be derived from an Abstract Base Class.

As an alternative to your factory function, you could specify a dict, mapping to your registrar classes:

Registrar = {'test.com': RegistrarA, 'test.biz': RegistrarB}

Then:

registrar = Registrar['test.com'](domain)

One quibble: You're not really doing a Class Factory here as you're returning instances rather than classes.

Jeff Bauer
  • 13,890
  • 9
  • 51
  • 73
10

In Python you can change the actual class directly:

class Domain(object):
  def __init__(self, domain):
    self.domain = domain
    if ...:
      self.__class__ = RegistrarA
    else:
      self.__class__ = RegistrarB

And then following will work.

com = Domain('test.com') #load RegistrarA
com.lookup()

I'm using this approach successfully.

bialix
  • 20,053
  • 8
  • 46
  • 63
  • See comments to http://stackoverflow.com/a/9144059/336527 for a warning (you are safe if all registrars have the same base class and don't use slots). – max Oct 21 '12 at 02:53
  • 37
    Actually, this approach carries a [much more serious danger](http://docs.python.org/reference/datamodel.html#id5) than I realized: special methods may not get called correctly, etc. I'm now convinced that this should NEVER be done, since the mess of figuring out what problems this may cause may vary with the version of Python, and is just not worth whatever benefits this provides. – max Oct 25 '12 at 06:50
  • 1
    Looks very hacky and not really different from using plain function. – serge.v Sep 25 '21 at 12:45
9

You can create a 'wrapper' class and overload its __new__() method to return instances of the specialized sub-classes, e.g.:

class Registrar(object):
    def __new__(self, domain):
        if ...:
            return RegistrarA(domain)
        elif ...:
            return RegistrarB(domain)
        else:
            raise Exception()

Additionally, in order to deal with non-mutually exclusive conditions, an issue that was raised in other answers, the first question to ask yourself is whether you want the wrapper class, which plays the role of a dispatcher, to govern the conditions, or it will delegate it to the specialized classes. I can suggest a shared mechanism, where the specialized classes define their own conditions, but the wrapper does the validation, like this (provided that each specialized class exposes a class method that verifies whether it is a registrar for a particular domain, is_registrar_for(...) as suggested in other answers):

class Registrar(object):
    registrars = [RegistrarA, RegistrarB]
    def __new__(self, domain):
        matched_registrars = [r for r in self.registrars if r.is_registrar_for(domain)]

        if len(matched_registrars) > 1:
            raise Exception('More than one registrar matched!')
        elif len(matched_registrars) < 1:
            raise Exception('No registrar was matched!')
        else:
            return matched_registrars[0](domain)
Ion Lesan
  • 317
  • 3
  • 4
  • Your first example is exactly what I developed on my own; however, this is the only place I've found it done this way. Do you know of any drawbacks to doing it like that? – Tom Aug 29 '16 at 22:00
  • 1
    It's difficult to tell. If you check the documentation https://docs.python.org/2/reference/datamodel.html#object.__new__, there's nothing there to discourage this usage, but not much to support it either. – Ion Lesan Aug 31 '16 at 03:22
  • 1
    Although it mentions a typical implementation, as well as what it was intended for (i.e. mainly for immutable classes), the possibility of `__new__` returning something different than an instance of `cls` is mentioned, too, and because returning `None` is explicitely forbidden, it would lead to the conclusion that an instance of a different class is allowed to be returned. – Ion Lesan Aug 31 '16 at 03:29
  • Thank, Ion. I ended up finding a [few](http://stackoverflow.com/a/5961102/1747771) [other](http://stackoverflow.com/a/8106977/1747771) [examples](http://whilefalse.net/2009/10/21/factory-pattern-python-__new__/), though it's not always [well-received](http://myownhat.blogspot.com/2012/03/polymorphism-beyond-factory-method.html). – Tom Aug 31 '16 at 14:58
4

I have this problem all the time. If you have the classes embedded in your application (and its modules) then you can use a function; but if you load plugins dynamically, you need something more dynamic -- registering the classes with a factory via metaclasses automatically.

Here is a pattern I'm sure I lifted from StackOverflow originally, but I don't still have the path to the original post

_registry = {}

class PluginType(type):
    def __init__(cls, name, bases, attrs):
        _registry[name] = cls
        return super(PluginType, cls).__init__(name, bases, attrs)

class Plugin(object):
    __metaclass__  = PluginType # python <3.0 only 
    def __init__(self, *args):
        pass

def load_class(plugin_name, plugin_dir):
    plugin_file = plugin_name + ".py"
    for root, dirs, files in os.walk(plugin_dir) :
        if plugin_file in (s for s in files if s.endswith('.py')) :
            fp, pathname, description = imp.find_module(plugin_name, [root])
            try:
                mod = imp.load_module(plugin_name, fp, pathname, description)
            finally:
                if fp:
                    fp.close()
    return

def get_class(plugin_name) :
    t = None
    if plugin_name in _registry:
        t = _registry[plugin_name]
    return t

def get_instance(plugin_name, *args):
    return get_class(plugin_name)(*args)
Mayur Patel
  • 945
  • 1
  • 7
  • 15
1

how about something like

class Domain(object):
  registrars = []

  @classmethod
  def add_registrar( cls, reg ):
    registrars.append( reg )

  def __init__( self, domain ):
    self.domain = domain
    for reg in self.__class__.registrars:
       if reg.is_registrar_for( domain ):
          self.registrar = reg  
  def lookup( self ):
     return self.registrar.lookup()    

Domain.add_registrar( RegistrarA )
Domain.add_registrar( RegistrarB )

com = Domain('test.com')
com.lookup()
Coyo
  • 616
  • 1
  • 5
  • 4
0

Since the methods are probably shared, using some base class would make sense. getattr can be used in the factory function to dynamically call another class.

The logic to figure out the registrartype should not be part these classes, but should be in some helper function.

import sys

class RegistrarBase():
    """Registrar Base Class"""
    def __init__(self, domain):
        self.name = domain

    def register(self, info):
        pass

    def lookup(self):
        pass
    def __repr__(self):
        return "empty domain"


class RegistrarA(RegistrarBase):
    def __repr__(self):
        return ".com domain"
                    
class RegistrarB(RegistrarBase):
    def __repr__(self):
        return ".biz domain"


def create_registrar(domainname, registrartype):
    try: 
        registrar = getattr(sys.modules[__name__], registrartype)
        return registrar(domainname)
    except:
        return RegistrarBase(domainname)

    
    
domain = create_registrar(domainname = 'test.com', registrartype='RegistrarA')    

print(domain)    
print(domain.name)
#.com domain
#test.com
Alex
  • 5,759
  • 1
  • 32
  • 47
  • It is a bit different from what OP it trying to do: his each derived class can identify its domain. You propose some kind of text table with domain/class name correspondence. – serge.v Sep 25 '21 at 12:57
0

Okay, here is an answer based on the answer of Alec Thomas, modified and extended: taking care of multi-level inheritance and ambiguity. If _resolve should be something more complicated than simple check of uniqueness and is likely to change it may be supplied as an argument and not be a class method.

base class module bbb.py:

from __future__ import annotations

from abc import ABC, abstractmethod
from typing import Sequence, Type


class Base(ABC):

    def __init__(self, *args, **kwargs):
        ...

    @classmethod
    def isit(cls, _s: str) -> bool:
        return False

    @classmethod
    def from_str(cls, s: str, *args, **kwargs) -> Base:
        subs = cls._findit(s)
        sc = cls._resolve(s, subs)
        return sc(*args, **kwargs)

    @classmethod
    def _findit(cls, s: str) -> Sequence[Type[Base]]:
        subs = [cls] if cls.isit(s) else []
        subs += [ssc for sc in cls.__subclasses__() for ssc in sc._findit(s)]
        return subs

    @classmethod
    def _resolve(cls, s: str, subs: Sequence[Type[Base]]) -> Type[Base]:
        if len(subs) == 0:
            raise Exception(f'Cannot find subclass for {s}')

        if len(subs) > 1:
            raise Exception(
                f'Cannot choose unique subclass for {s}: {subs}')
        sc = subs[0]
        return sc


class B(Base):
    @classmethod
    def isit(cls, s: str) -> bool:
        res = s == 'b class'
        return res
    enter code here

derived class module ccc.py:

from bbb import Base


class C(Base):
    @classmethod
    def isit(cls, s: str) -> bool:
        res = s == 'c class'
        return res


class CC(Base):
    @classmethod
    def isit(cls, s: str) -> bool:
        res = s == 'cc class'
        return res

How to use:

In [4]: from bbb import Base

In [5]: import ccc

In [6]: Base.from_str('b class')
Out[6]: <bbb.B at 0x1adf2665288>

In [7]: Base.from_str('c class')
Out[7]: <ccc.C at 0x1adf266a908>

In [8]: Base.from_str('cc class')
Out[8]: <ccc.CC at 0x1adf2665608>
serge.v
  • 1,854
  • 1
  • 11
  • 7
-1

Here a metaclass implicitly collects Registars Classes in an ENTITIES dict

class DomainMeta(type):
    ENTITIES = {}

    def __new__(cls, name, bases, attrs):
        cls = type.__new__(cls, name, bases, attrs)
        try:
            entity = attrs['domain']
            cls.ENTITIES[entity] = cls
        except KeyError:
            pass
        return cls

class Domain(metaclass=DomainMeta):
    @classmethod
    def factory(cls, domain):
        return DomainMeta.ENTITIES[domain]()

class RegistrarA(Domain):
    domain = 'test.com'
    def lookup(self):
        return 'Custom command for .com TLD'

class RegistrarB(Domain):
    domain = 'test.biz'
    def lookup(self):
        return 'Custom command for .biz TLD'


com = Domain.factory('test.com')
type(com)       # <class '__main__.RegistrarA'>
com.lookup()    # 'Custom command for .com TLD'

com = Domain.factory('test.biz')
type(com)       # <class '__main__.RegistrarB'>
com.lookup()    # 'Custom command for .biz TLD'