Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE]: Fix error handling and stop using unnecessary exceptions #819

Open
Tgenz1213 opened this issue Nov 12, 2024 · 0 comments
Open
Labels
enhancement New feature or request

Comments

@Tgenz1213
Copy link
Collaborator

Feature summary

Exceptions are not meant for flow control

Feature description

Exceptions should be reserved for truly exceptional cases. There are many occurrences of exception abuse, overusing try-blocks, and excepting Exception when a function throws a specific exception.

Non-compliant

def _unfollow_company(self) -> None:
    try:
        logger.debug("Unfollowing company")
        follow_checkbox = self.driver.find_element(
            By.XPATH, "//label[contains(.,'to stay up to date with their page.')]")
        follow_checkbox.click()
    except Exception as e:
        logger.debug(f"Failed to unfollow company: {e}")

Compliant

# compliant
def _unfollow_company(self) -> None:
    logger.debug("Unfollowing company") # Does not throw exception - has no reason to be in the try-block

    # Handle one function that throws an error at a time
    try:
        # find_element() only throws a NoSuchElementException - there's no reason to catch a general exception
        follow_checkbox = self.driver.find_element(By.XPATH, "//label[contains(.,'to stay up to date with their page.')]")
    except NoSuchElementException as e:
        logger.debug(f"Failed to unfollow company: {e}")

    # Does not throw exception - has no reason to be in the try-block
    follow_checkbox.click()

Motivation

It significantly improves maintainability and readability

Alternatives considered

No, the current approach to error handling and exception abuse as flow control is unacceptable.

Additional context

No response

@Tgenz1213 Tgenz1213 added the enhancement New feature or request label Nov 12, 2024
Tgenz1213 pushed a commit to Tgenz1213/Auto_Jobs_Applier that referenced this issue Nov 13, 2024
…ve error handling and formatting (fix previous merge mistakes)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: No status
Development

No branches or pull requests

1 participant