3
\$\begingroup\$

I forked this repo to be more concise. The code is here. I'll paste it below since that seems to be the style. I removed the class definitions at the bottom that I didn't change -- the edit I'm concerned with is the use of the "class_factory" function at the bottom.

Is this good? Pythonic?

from selenium.webdriver import DesiredCapabilities
from selenium.webdriver.firefox.webdriver import WebDriver as _Firefox
from selenium.webdriver.chrome.webdriver import WebDriver as _Chrome
from selenium.webdriver.ie.webdriver import WebDriver as _Ie
from selenium.webdriver.remote.webdriver import WebDriver as _Remote
from selenium.webdriver.phantomjs.webdriver import WebDriver as _PhantomJS

from webdriverplus.utils import _download
from webdriverplus.webdriver import WebDriverDecorator
from webdriverplus.webelement import WebElement

import atexit
import os
import socket
import subprocess
import time

try:
    from urllib2 import URLError
except ImportError:
    from urllib.error import URLError

VERSION = (0, 2, 0)


def get_version():
    return '%d.%d.%d' % (VERSION[0], VERSION[1], VERSION[2])

class WebDriver(WebDriverDecorator):
    _pool = {}  # name -> (instance, signature)
    _quit_on_exit = set()  # set of instances
    _selenium_server = None  # Popen object
    _default_browser_name = 'firefox'

    @classmethod
    def _at_exit(cls):
        """
        Gets registered to run on system exit.
        """
        if cls._selenium_server:
            cls._selenium_server.kill()

        for driver in cls._quit_on_exit:
            try:
                driver.quit(force=True)
            except URLError:
                pass

    @classmethod
    def _clear(cls):
        cls._pool.clear()

    @classmethod
    def _get_from_pool(cls, browser):
        """Returns (instance, (args, kwargs))"""
        return cls._pool.get(browser, (None, (None, None)))

    def __new__(cls, browser=None, *args, **kwargs):
        browsers = {'firefox':Firefox, 
                     'chrome':Chrome,
                     'ie':Ie,
                     'remote':Remote,
                     'phantomjs':PhantomJS,
                     'htmlunit':HtmlUnit}

        quit_on_exit = kwargs.get('quit_on_exit', True)
        reuse_browser = kwargs.get('reuse_browser')
        signature = (args, kwargs)

        browser = browser or cls._default_browser_name
        reused_pooled_browser = False
        pooled_browser = None

        try:
            is_str = isinstance(browser, basestring)
        except NameError:
            is_str = isinstance(browser, str)
        if is_str:
            browser = browser.lower()
            pooled_browser, pooled_signature = WebDriver._get_from_pool(browser)

            if pooled_signature == signature:
                driver = pooled_browser
                reused_pooled_browser = True
            elif browser in browsers.keys():
                driver = browsers[browser](*args, **kwargs)
            else:
                raise BrowserNotSupportedError()

        # If a WebDriverDecorator/WebDriver is given, add it to the pool
        elif isinstance(browser, WebDriverDecorator):
            driver = browser
            browser = driver.name
        else:
            kwargs['driver'] = browser
            driver = WebDriverDecorator(*args, **kwargs)
            browser = driver.name

        if reuse_browser and not reused_pooled_browser:
            if pooled_browser:
                pooled_browser.quit(force=True)
            WebDriver._pool[browser] = (driver, signature)

        if quit_on_exit:
            WebDriver._quit_on_exit.add(driver)

        return driver

    def __init__(self, browser='firefox', *args, **kwargs):
        pass
        # Not actually called.  Here for autodoc purposes only.

atexit.register(WebDriver._at_exit)


browser_types = ({'name':'Firefox', 'driver':_Firefox},
                 {'name':'Chrome', 'driver':_Chrome},
                 {'name':'Ie', 'driver':_Ie},
                 {'name':'Remote', 'driver':_Remote},
                 {'name':'PhantomJS', 'driver':_PhantomJS},)

def class_factory(browser_type, bases):
    class Class_(*bases):
        def __init__(self, *args, **kwargs):
            kwargs['driver'] = browser_type['driver']
            super().__init__(*args, **kwargs)
    Class_.__name__ = browser_type['name']
    return Class_

for browser_type in browser_types:
    globals()[browser_type['name']] = class_factory(browser_type, 
                                                   (WebDriverDecorator,))
\$\endgroup\$
2
\$\begingroup\$

It's not unknown: python is really good at this, although the more common approach would be to use a metaclass. The immediate drawbacks are

1) it introduces a state-changing dependency to the import statement. If code that is using this code gets imported in a non-standard way you may get confusing errors because types will or will not appear depending on when this module gets run. It's not a major issue if this code will be imported directly but it's potentially problematic if there is more magic going on elsewhere.

2) less importantly, it's going to play hell with IDE's that try to do autocomplete for you :)

I'm guessing that the super().init idiom is a python 3 replacement for type('Name', (),{}) by it's form. If it's not - that is the old way to create a runtime type and it avoids creating and renaming the Class_ class, which seems messy to me. Examples of the 'old way' here

Lastly: this seems like classes that differ only in data, or to be more precise in composition. In cases like that I've always found it more maintainable to do it declaratively with class-level variables and appropriate indirections:

class Browser(object):
   BROWSER = 'browser'
   DRIVER = None

   @property
   def name(self):
       return self.BROWSER

   def do_something(self):
       self.DRIVER.do_something()


class Firefox(Browser):
    BROWSER = 'Firefox'
    DRIVER = _Firefox

class Chrome (Browser):
    BROWSER = 'Chrome'
    DRIVER = _Chrome

Doing this allows you to do subclassing and overrides as appropriate, which is much hairier with types that have to be created before they can be changed.

\$\endgroup\$
6
  • \$\begingroup\$ Interesting, thank you for the feedback. Some responses (split across a few comments): \$\endgroup\$ – Patrick Collins Aug 23 '13 at 15:42
  • \$\begingroup\$ super().__init__() is Python 3's way of saying (as I understand it) "walk up my inheritance tree until you find a parent class with an __init__ method, and then call it with the specified arguments". I saw type() but avoided it because I found it difficult to specify __init__ correctly inside of it. I tried defining it as a curried function init(*args, **kwargs) that returned a function __init__(self, *args, **kwargs) and then doing something like type('Firefox', _Firefox, dict(__init__ = init(*args, **kwargs))) but I don't think self resolved correctly. \$\endgroup\$ – Patrick Collins Aug 23 '13 at 15:42
  • 1
    \$\begingroup\$ self should work without special currying as long as the functions being called exist in scope when you call type:in Python 2.7, at least: def blah(self): self.x = '123' fred = type('fred', (), {'init':blah}) fred().x '123' \$\endgroup\$ – theodox Aug 23 '13 at 16:26
  • \$\begingroup\$ Also, the way you recommend is basically what the original code looks like. I started going in this direction because I needed to subclass all of the browser-related classes in another module, and it seemed wrong to me to copy+paste six different class definitions. I thought it might make the original module cleaner too. Your response suggests, though, that this is poor style. The link you posted about type() mentions that "If you wonder whether you need [metaclasses], you don't." Should I avoid them, then, unless I find myself in a situation where they offer a substantially larger benefit? \$\endgroup\$ – Patrick Collins Aug 23 '13 at 16:35
  • 1
    \$\begingroup\$ They are great when there is no reasonable alternative - if you're completely commiting to a fluid runtime environment, its's a cool thing to have. But in a case like this, where the problem domain is basically fixed, it's nice to have the inspectablility and maintainability (and opportunities for documentation!!!) that you get from predefined types. You can also write code to generate code files of conventional types when you have a problem which involves a lot of repetition \$\endgroup\$ – theodox Aug 23 '13 at 16:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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