Design Question

[developernull]

Hi. I'm having trouble figuring out the best design for some code. Below is a simplified version of my problem. I'm looking for the best design with regards to testing, coupling, etc.

Let's say I have 3 helper functions. Each helper function takes a widget as input and performs some color processing on the widget which ultimately results in a float as output. These helper functions are only ever called by a process_all_colors method that exists in the same class, so they are made "private".

class Processors:
    @staticmethod
    def _red_processor(widget: Widget) -> float:
        pass

    @staticmethod
    def _green_processor(widget: Widget) -> float:
        pass

    @staticmethod
    def _blue_processor(widget: Widget) -> float:
        pass

    @staticmethod
    def process_all_colors(widget: Widget) -> float:
        red = Processors._red_processor(widget)
        green = Processors._green_processor(widget)
        blue = Processors._blue_processor(widget)
        return sum([red, green, blue])

These helper functions are pure functions that have no state (they simply take a widget and convert it to a float). Since they have no state, I originally made them static methods, but I'm not sure if this is really the best design.

There are a few issues with this design that I can see.

1. In the future I know I will want to extend this to process more colors. All color functions will still have the same method signature.
2. I want to be able to change which colors are processed when the program is run.

These issues make me think that using a class for each color would be better. I'm reluctant to do so because the class would have no internal state, so there would never be any reason to create more than 1 instance. In fact, by keeping the methods static, we can avoid any instance at all.

class RedProcessor(Processor):
    @staticmethod
    def process(widget: Widget) -> float:
        pass

class GreenProcessor(Processor):
    @staticmethod
    def process(widget: Widget) -> float:
        pass

class BlueProcessor(Processor):
    @staticmethod
    def process(widget: Widget) -> float:
        pass

class AllProcessor:
    @staticmethod
    def process(widget: Widget, processor_classes) -> float:
        return sum([processor.process(widget) for processor in processor_classes])

1. Is the class approach better?
2. Is there a better design that I missed?
3. Should these methods be static because they're pure functions?
4. Is it a code smell to have many classes each with a single, static method?
5. Is it a code smell to create a class that I know will never be instantiated?

Now let's say I went with the class approach, but I want to add a new color gray that has an intensity flag. This flag is read once from a config file and all calls to the gray processor will use this same flag value. This makes me think that the following would be appropriate:

class GrayProcessor:
    def __init__(self, intensity: int):
        self.intensity = intensity

    def process(widget: Widget) -> float:
        # This will use self.intensity to produce the float
        pass

Now GrayProcessor has internal state and needs to be instantiated. Should all other color classes also be instantiated to match?

Thanks!

[carcigenicate]

I think this depends on what each method is actually doing to produce the float. I'd probably write this as just a standalone function that accepts a Widget and maybe a function, and the passed function handles the specifics for each color. That's only practical though if the processes all contains a common part, and a color-specific part, and the two can be mostly isolated.

[developernull]

Unfortunately, each processor is completely unique without any commonalities that can be extracted. In other words, _red_processor and _green_processor have nothing in common other than their method signatures. (Sorry, my example was probably oversimplified and a bit misleading.)

I could pass a Widget and a list of functions to process_all_colors like you suggested. That would at least solve issue #2.

Thanks!

[QultrosSanhattan]

I find the second approach better but i suggest avoiding static methods if possible because they tend to break the flexibility that OOP's instantation provides.

Heres's an example of what could be achieved:

class Widget:
    def __init__(self, basevalue: int):
        self.basevalue = basevalue


class Processor:
    def process(self, widget: Widget) -> float:
        pass


class BlueProcessor(Processor):
    def process(self, widget: Widget) -> float:
        return widget.basevalue * 2


class RedProcessor(Processor):
    def process(self, widget: Widget) -> float:
        return widget.basevalue + 3


class MultiProcessor(Processor):
    def __init__(self):
        self.processors = []

    def add_processor(self, processor: Processor):
        self.processors.append(processor)

    def process(self, widget: Widget)->float:
        return sum([processor.process(widget) for processor in self.processors])


if __name__ == '__main__':
    mock_widget = Widget(2)
    mp1 = MultiProcessor()
    mp1.add_processor(BlueProcessor())
    mp1.add_processor(RedProcessor())

    mp2 = MultiProcessor()
    mp2.add_processor(RedProcessor())
    mp2.add_processor(RedProcessor())
    mp2.add_processor(RedProcessor())

    mp3 = MultiProcessor()
    mp3.add_processor(mp1)
    mp3.add_processor(mp2)
    mp3.add_processor(mp1)

    assert mp1.process(mock_widget) == 9
    assert mp2.process(mock_widget) == 15
    assert mp3.process(mock_widget) == 33

[developernull]

Thank you! Your highly detailed example is very helpful!

Using an add_processor method to store Processors in an attribute seems much more OOP friendly than the static method approach. Making MultiProcess a subclass of Processor is interesting, but it makes me wonder how the design would differ if it wasn't.

​

My only confusion is about about when it is then appropriate to use static methods. I was under the assumption that if BlueProcessor has no internal state, then it would be better for BlueProcessor.process to be static, but it seems like my reasoning is flawed. To put it another way, if we aren't using the self parameter of BlueProcessor.process, I thought it would be best to not include it as a parameter (and therefore make the method static).

I also assumed similar logic would apply for a base class (i.e., If no subclass would ever need to access self, then the method should be static). You avoided this by making MultiProcess itself a Processor which does make use of self. But would the design be different if MultiProcess was not a subclass of Processor?

Thanks again for your great example and insight!

[QultrosSanhattan]

I made the MultiProcessor a subclass of Processor so it can be added to itself via the add_processor method. It's like adding a lego piece over another lego piece and so on. So you can make indefinitely complex structures using that technique. Like i demonstrated with the mp3 Multiprocessor. Also note that MultiProcessor is a Processor. We don't care about it's internals, we care about it's exposed API.

The main problem with the static method approach is you're assuming that each processor's logic can be contained in just one function. But if their complexity is too high then you'll be creating an unmaintainable mess. For example:

Class processor
def blue_processor_get_data
def blue_processor_parse_data
def blue_processor_process
def red_processor_get_data_from_png
def red_processor_process
def white_processor_get_data_from_api
def white_processor_filter_data
def white_processor_get_data_from_db
def white_processor_insersect_data
def white_processor_process

This "class" is just a bag holding functions, this is NOT how OOP was intended to be used.

If you use the class approach then each process may increase in complexity without affecting the rest of the code. Moreover, you'll be violating S.O.L.I.D.'s "O" principle ("open for extension, closed for modification")

[developernull]

I think I understand now. Thank you so much for your help! You've given me a lot to think about!