How to refactor a lot of Functions that can be run in two different modes

Question

I have some code and in it there are functions which can be run in one of two modes (in my case server mode and local mode). For example, most functions look something like this:

def path_join(path_elements,run_type='local'):
    if run_type == 'local':
        path = os.path.join(*path_elements).replace("/", os.path.sep)
    elif run_type == 'blob':
        path = clean_path('/'.join(path_elements))
    return path

def clean_path(path: str, run_type='local') -> str:
    """
    Replaces repeated path separators in a path.
    """
    if run_type == 'local':
        separator = get_os_path_sep()
    elif run_type == 'blob':
        separator = constants.BLOB_PATH_SEP

    separator = separator * 2 if separator == constants.WINDOWS_PATH_SEP else separator
    path = re.sub(fr"{separator}+", f"{separator}", path)
    return path

def read_text(filename,run_type,container):
    root = get_root_path()
    if run_type == 'local':
        with open(filename, 'r') as file:
            return file.read()
    elif run_type == 'blob':
        stream = read_blob(container, blob_path, run_type, use_lease=True)
        return stream.read().decode('utf-8')

I'm trying to refactor this code because it feels redundant to check for case in all these functions. One option I guess is to break down the functions and have the and create a class to decide which variation of the function to call. The problem is that all the functions don't have too much in common so I think in the end I would just have a lot of small classes which actually may be more complex (but maybe I'm wrong OOP is still fresh to me). Does anyone have any advice on how to make this better?

Answer 1

If the value of run_type can be determined before these are called then classes may be a good solution.

Currently it looks like you call the code this way:

read_text( clean_path( path_join(path, run_type), run_type), run_type, container)

When it could be:

run_type.read_text(path, container)

This would use polymorphism to call different read_text methods based on the type of run_type.

You may be reluctant to “create a lot of small classes” (in this case it'd be two, for now) but that is typical of OOP. It's a very different style. Rather then pull details towards you and try to know everything, you push details behind abstractions so you can forget about them.

Of course OOP, classes, and the strategy patern are hardly the only way to solve this problem. This question reminds me of the Boolean argument code smell. You can find example solutions for that here and here.

Answer 2

I like to look at how usable the abstraction is from the perspective of the consuming client first, and only second from the perspective of its internal implementation.

If you already have a sizeable code base, what I would do is to look at the code that is consuming this (set of) abstraction(s).

Do calling consumers pass constants or variables for that parameter?

• At one extreme, if users of these methods always pass constants, then those are really two different methods that should simply have two different names, and the parameter should be removed from both.

• At the other extreme, if users never pass constants, always a variable, then it makes sense to keep a single signature with a differentiating parameter.

Of course, we can do both, if there are significant uses of constants being passed as well as uses of the variable, where one implementation (e.g. taking the variable or separate methods) can be written in terms of the other(s), and vice versa, without necessarily affecting the consuming clients.

Answer 3

this looks like you need to configure the calls before you use the functions. This is done by "dependency injection" in OOP or callbacks/"higher order functions" in FP. What you do is you abstract away what is inside of the if blocks and then construct the function and return it. I am not super familiar how you would solve this in python (in JS world you have module.exports/export that make this easier).

def _path_join_local(path_elements):
    return os.path.join(*path_elements).replace("/", os.path.sep)
    
def _path_join_blob(path_elements):
    return _clean_path('/'.join(path_elements))

def _get_separator_local():
    get_os_path_sep()

def _get_separator_blob():
    constants.BLOB_PATH_SEP

def _get_separator(system_separator):
    return system_separator * 2 if system_separator == constants.WINDOWS_PATH_SEP else system_separator

# separator can be determined at the start
# Yes... I know this is ugly and over engineered
sparator = _get_separator(_get_separator_local() if run_type == 'local' else _get_separator_blob())

def _clean_path(path: str) -> str:
    return re.sub(fr"{separator}+", f"{separator}", path)

def _read_text_local(filename):
    with open(filename, 'r') as file:
        return file.read()

def _read_text_blob(filename):
    # I have no idea if container is a parameter or if you can get it here
    container = ...
    stream = read_blob(container, blob_path, use_lease=True)
    return stream.read().decode('utf-8')

public = {
    "path_join": _path_join_local if run_type == 'local' else _path_join_blob,
    "clean_path": _clean_path,
    "read_text": _read_text_local if run_type == 'local' else _read_text_blob,
}    

The optimist in me would now expect the above used as follows:

from pkg import public as util

util.path_join(["aesd", "unqwfav"])
util.read_text("jazbec")

The super optimistic claim would be that the following would be possible:

from pkg.public import path_join, read_text

...

but I don't think that is how python works.

Anyways, this is the idea: have public object configured on startup by some sort of a factory (the factory "methods" in this case are the ternary expressions defining the public object properties). Hopefully this makes sense, but I feel like it really overcomplicates the code. It might be of some help if these were split into multiple files.

Answer 4

In OOP style, you would create a class that contains all the functions let say for server mode. Then you derive a new class from the first, that inherits all the functions. Here you can overwrite the functions that are different for local mode.

Now you can create instances (objects) from the one or other class, depending on your mode. That way you always call the right function without decision.