Improve code maintainability and readability in src/clevercloud_storage_framework/client.py #8

New issue

Open

opened 2025-09-29 20:33:37 +00:00 by CoreRasurae · 0 comments

CoreRasurae commented

2025-09-29 20:33:37 +00:00

Member

Kyle suggested that: Many of the code changes here are not complete repetitions introduced in PR #6, but are still very similar to one another. It might be more maintainable to define a function like:

def _execute_function_on_provider(self, path, funcname, *args, **kwargs):
    # Obtain a suitable provider for the path if it exists
    source_provider = self._helper_find_provider_if_needed(path)

    if source_provider != self.provider:
        try:
            return getattr(source_provider, funcname)(*args, **kwargs)
        except StorageError:
            # Always fall back to the current provider
        finally:
            if source_provider:
                source_provider.close()

    return getattr(self.provider, funcname)(*args, **kwargs)

Then you can call:

list(self._execute_func_on_provider('list_files', path, recursive)) in list_files
self._execute_func_on_provider('file_exists', path) in file_exists
self._execute_func_on_provider('get_file_size', path) in get_file_size

and so on.

The following code block can be made more readable

   else:
                source_provider = StorageClientFactory.get_provider_for_path(path)

        return self.provider if source_provider is None else source_provider

with:
Idiomatic Python might prefer return source_provider or self.provider

Tasks to be done:

Apply Kyle suggestions as is or similar approach if the suggested reflection incurs a performance penalty.

Definition of done:

Show that code is more maintainable and that it still passes the tests and coverage

- Kyle suggested that: Many of the code changes here are not complete repetitions introduced in PR #6, but are still very similar to one another. It might be more maintainable to define a function like: ``` def _execute_function_on_provider(self, path, funcname, *args, **kwargs): # Obtain a suitable provider for the path if it exists source_provider = self._helper_find_provider_if_needed(path) if source_provider != self.provider: try: return getattr(source_provider, funcname)(*args, **kwargs) except StorageError: # Always fall back to the current provider finally: if source_provider: source_provider.close() return getattr(self.provider, funcname)(*args, **kwargs) ``` Then you can call: ``` list(self._execute_func_on_provider('list_files', path, recursive)) in list_files self._execute_func_on_provider('file_exists', path) in file_exists self._execute_func_on_provider('get_file_size', path) in get_file_size ``` and so on. - The following code block can be made more readable ``` else: source_provider = StorageClientFactory.get_provider_for_path(path) return self.provider if source_provider is None else source_provider ``` with: Idiomatic Python might prefer `return source_provider or self.provider` Tasks to be done: - Apply Kyle suggestions as is or similar approach if the suggested reflection incurs a performance penalty. Definition of done: - Show that code is more maintainable and that it still passes the tests and coverage

CoreRasurae changed title from ~~Improve code maintainability and readability in~~ to Improve code maintainability and readability in src/clevercloud_storage_framework/client.py

2025-09-29 20:34:16 +00:00

CoreRasurae added the

labels

2025-09-29 20:37:54 +00:00

CoreRasurae added this to the v0.1 milestone

2025-09-29 20:38:14 +00:00