Search code examples
pythonrefactoringrepeatdrytry-except

Is there a way to refactor this try/except block to avoid DRY violation?


This is part of a larger function using Selenium with Python for a web automation script. The website sometimes serves a popup, which results in an ElementClickInterceptedException but the popup isn't always served; sometimes there is no popup.

I wrote this function to close the popup if it's served:

def close_popup():
    try:
        WebDriverWait(driver, 5).until(EC.presence_of_element_located((By.XPATH, '//x-button'))).click()
        time.sleep(1)
    except:
        NoSuchElementException
        print("No pop-up found")

Then I wrote a try/except block in the main section which calls the function:

try:
    WebDriverWait(driver, 10).until(EC.presence_of_element_located((By.LINK_TEXT, "Click here!"))).click()
except ElementClickInterceptedException:
    close_popup()
    WebDriverWait(driver, 10).until(EC.presence_of_element_located((By.LINK_TEXT, "Click here!"))).click()

This does work as expected, but it violates DRY (Don't Repeat Yourself), since the line after try: is the same as the last line of the block.

I did search this first but did not find any to be exactly like my question. Those I found were asking about some error in the code. My code does work, it just violates DRY.

How would I refactor so that my code is Pythonic and does not violate DRY?


Solution

  • DRY mostly pertains to keeping your business logic in one place; i.e. not repeat logical decisions all over the place, like this one:

    if len(user.subaccounts) > 3 and user.license == 'basic':
        raise UpgradeSubscription
    

    There should be exactly one place this kind of thing is checked, not multiple places throughout your codebase. Because that will lead to inconsistencies when your business rules eventually inevitably change.

    So, there's no violation of DRY here in particular; your two lines of code are very close together, and if you change one, you'll probably have little issue copy-pasting it three lines below.

    Having said that, the obvious choices are:

    def close():
        WebDriverWait(driver, 10).until(EC.presence_of_element_located((By.LINK_TEXT, "Click here!"))).click()
    
    try:
        close()
    except ElementClickInterceptedException:
        close_popup()
        close()
    

    or something like:

    for i in range(2):
        try:
            WebDriverWait(driver, 10).until(EC.presence_of_element_located((By.LINK_TEXT, "Click here!"))).click()
        except ElementClickInterceptedException:
            if i == 0:
                close_popup()
            else:
                raise
    

    Though the former one is arguably more readable.