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

implement early returns #1276

Open
3 tasks done
miloth opened this issue Dec 6, 2024 · 1 comment
Open
3 tasks done

implement early returns #1276

miloth opened this issue Dec 6, 2024 · 1 comment

Comments

@miloth
Copy link

miloth commented Dec 6, 2024

Prerequisites

  • Are you running the latest version of this application?
  • Have you checked the Frequently Asked Questions document?
  • Did you perform a cursory search of other issues to look for related issues?

Feature Request

Description

The methods do not always use the early return pattern.

Desired Behavior of Feature

Foe example this method now written as:

    def __scan_files_if_no_errors(
        self,
        args: argparse.Namespace,
        use_standard_in: bool,
        files_to_scan: List[str],
        did_error_scanning_files: bool,
    ) -> ApplicationResult:  # sourcery skip: extract-method
        scan_result = ApplicationResult.SUCCESS
        did_fix_any_files = False
        if did_error_scanning_files:
            self.__handle_error("No matching files found.", None, exit_on_error=False)
            scan_result = ApplicationResult.NO_FILES_TO_SCAN
        else:
            POGGER.info("Initializing parser.")
            self.__initialize_parser()

            POGGER.info("Processing files with parser.")
            assert (
                self.__tokenizer is not None
            ), "When scanning, the tokenizer should have already been initialized."
            fsh = FileScanHelper(
                self.__tokenizer,
                self.__plugins,
                self.__presentation,
                self.__show_stack_trace,
                self.__handle_error,
            )
            did_fix_any_files, did_fail_any_file = fsh.process_files_to_scan(
                args, use_standard_in, files_to_scan, self.__string_to_scan
            )
            if did_fail_any_file:
                scan_result = ApplicationResult.SYSTEM_ERROR
            elif did_fix_any_files:
                scan_result = ApplicationResult.FIXED_AT_LEAST_ONE_FILE
            elif self.__plugins.number_of_scan_failures:
                scan_result = ApplicationResult.SCAN_TRIGGERED_AT_LEAST_ONCE
            POGGER.info("Files have been processed.")

        return scan_result

would be refactored as:

    def __scan_files_if_no_errors(
        self,
        args: argparse.Namespace,
        use_standard_in: bool,
        files_to_scan: List[str],
        did_error_scanning_files: bool,
    ) -> ApplicationResult:  # sourcery skip: extract-method
        if did_error_scanning_files:
            self.__handle_error("No matching files found.", None, exit_on_error=False)
            return ApplicationResult.NO_FILES_TO_SCAN

        POGGER.info("Initializing parser.")
        self.__initialize_parser()

        POGGER.info("Processing files with parser.")
        assert (
            self.__tokenizer is not None
        ), "When scanning, the tokenizer should have already been initialized."
        fsh = FileScanHelper(
            self.__tokenizer,
            self.__plugins,
            self.__presentation,
            self.__show_stack_trace,
            self.__handle_error,
        )
        did_fix_any_files, did_fail_any_file = fsh.process_files_to_scan(
            args, use_standard_in, files_to_scan, self.__string_to_scan
        )
        POGGER.info("Files have been processed.")

        if did_fail_any_file:
            return ApplicationResult.SYSTEM_ERROR
        if did_fix_any_files:
            return ApplicationResult.FIXED_AT_LEAST_ONE_FILE
        if self.__plugins.number_of_scan_failures:
            return ApplicationResult.SCAN_TRIGGERED_AT_LEAST_ONCE

        return ApplicationResult.SUCCESS

What Are the Benefits of This Feature?

The code becomes more readable and it would potentially benefit from speed improvements, as the code after the early returns would not be run.

What's the feeling about this? I wanted to ask before deciding whether it's worth doing a PR.

@jackdewinter
Copy link
Owner

I would be interested in more like this. Not so much worried about the performance aspect, as I have a set of performance tests ready to go, and just have to get some time to apply them.

Would you be willing to look for more ideas like this, where you think it would be more readable? Perhaps collect them in a PR so we could talk about them?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants