1

In writing a python object factory, I'm running into a lot of parameter repetition in the constructors. It feels wrong, like there is a better way to use this pattern. I'm not sure if I should be replacing the parameters with **kwargs or if there is a different design pattern that is more suited to this sort of case.

A simplified example is below. The real code is of course more complicated and you can see more reasons why I'd do it this way, but I think this is a reasonable Minimal Reproducible Example

External to these classes, for the API, the most important factors are species and subspecies. It happens to be that internally, is_salt_water is important and results in a different object, but that's an internal matter.


class Fish:
    def __init__(self, species, sub_species, length, weight):    # Repeating this a lot
        self.species = species
        self.sub_species = sub_species
        self.length = length
        self.weight = weight
        self.buoyancy = self._calc_buoyancy()

    def _calc_buoyancy(self):
        raise Exception("Do not call this abstract base class directly")


class FreshWaterFish(Fish):
    def __init__(self, species, sub_species, length, weight):    # Repeating this a lot
        self.fresh_water = True
        super().__init__(species, sub_species, length, weight)   # Repeating this a lot
    def _calc_buoyancy(self):
        self.buoyancy = 3.2 * self.weight   #totally made-up example. No real-world meaning


class SaltWaterFish(Fish):
    def __init__(self, species, sub_species, length, weight):    # Repeating this a lot
        self.fresh_water = False
        super().__init__(species, sub_species, length, weight)   # Repeating this a lot

    def _calc_buoyancy(self):
        self.buoyancy = 1.25 * self.weight / self.length  #totally made-up example. No real-world meaning

def FishFactory(species, sub_species, length, weight, is_salt_water = False): # Repeating this a lot
    mapper = {True : FreshWaterFish, False: SaltWaterFish}
    return mapper[is_salt_water](species, sub_species, length, weight) # Repeating this a lot
Mort
  • 3,379
  • 1
  • 25
  • 40
  • The usual solution is to use `*args, **kwargs` to accept and pass along general arguments. – Barmar Apr 28 '23 at 22:02
  • And I guess I need to check both `args[4]` and `kwargs['is_salt_water']` because it could be in either place? – Mort Apr 29 '23 at 19:20
  • Note that your usage of `_calc_bouyancy()` is inaccurate. Either call the function without assignment or change it to return the calculation instead of assigning. Also if I am not mistaken it appears that calling `super()` raises that abstract method exception. Use `pass` instead. – youngson May 09 '23 at 02:46

2 Answers2

1

Im not aware of better methods, but they might be first of all you can yes use *args, **kwargs but if you want the help from the compiler it suck if not done properly

i just try to avoid replacing __init__ at all

in some cases is just better to save a class variable (like in this case that fresh_water is the same for everyone)

class FreshWaterFish:
    fresh_water = True
    def buoyancy(self):
        self.buoyancy = 3.2 * self.weight 

or get the information inside an another function like

class Big:
    def __init__(self, a, l, o, t, o, f, args):
        ...
        self.init()

    def init(self):
        pass

class Small(Big):
      def init(self):
          self.dragon = False

My second choise is just to work with a dictionary as a single parameter for all the facoltative ones, so i can easily shamble it with kwargs*, and add a description like

def function(dictionary:dict):
    """:param dictionary: {name:str , age:int, money:FileNotFoundError}"""
    ...

hope it helps

kwargs*:

You can use a dictionary like the **kwargs argument

Doing this:

func(param1=value1, param2=value2, ecc...)

Is equal to:

params = dict(param1=value1, param2=value2, ecc...) # or {'param1':value1, ecc...} but, by me, is less readable
func(**params)
  • Yes. Forgoing the _ _ init _ _ is what I ended up doing. I actually hadn't seen your post and went to post that as my own answer when I saw yours. Thanks. – Mort May 22 '23 at 17:07
0

The trick is to use the Dependency Inversion Principle. Instead of providing the concrete implementation species, sub_species, length, weight, you should encapsulate them into an object and pass that object around instead. You have almost done that with the Fish class, but as you see, inheriting from Fish creates redundant parameter lists.

One option you have is to employ composition to simulate inheritance, which in turn fulfills the Dependency Inversion Principle. Here we wrap FreshWaterFish and SaltWaterFish around the Fish class. To keep the derived fish classes polymorphic, we separate the abstract methods into an interface. Effectively we separate behavior from data.

class Fish:
    def __init__(self, species, sub_species, length, weight):
        self.species = species
        self.sub_species = sub_species
        self.length = length
        self.weight = weight

class IFish:
    @abstractmethod
    def _calc_buoyancy(self):
        pass
    
    @abstractmethod
    def _swim(self):
        pass

class FreshWaterFish(IFish):
    def __init__(self, fish):
        self.fish = fish
        self.fresh_water = True
        self.buoyancy = self._calc_bouyancy()

    def _calc_buoyancy(self):
        return 3.2 * self.fish.weight

    def _swim(self):
        print('swim swim')


class SaltWaterFish(IFish):
    def __init__(self, fish):
        self.fish = fish
        self.fresh_water = False
        self.buoyancy = self._calc_bouyancy()

    def _calc_buoyancy(self):
        return 1.25 * self.fish.weight / self.fish.length

    def _swim(self):
        print('swam swam')

def FishFactory(self, species, sub_species, length, weight, is_salt_water = False):
    mapper = {True : FreshWaterFish, False: SaltWaterFish}
    fish_base = Fish(species, sub_species, length, weight)
    return mapper[is_salt_water](fish_base)

It is still inevitable to provide the long argument list somewhere but now the redundancy is minimized to where it makes sense and the code is rather scalable (assuming that the binary freshwater/saltwater is a semantic generalization).

When using inheritance, do consider if the 'X is a Y' relationship may not the best approach. Yes it semantically makes sense, but that thinking can produce practical issues in your code.

Sources:

  1. https://stackoverflow.com/a/51273196/8724072

See also:

youngson
  • 85
  • 10
  • Note if your use case requires inheritance of `Fish`, it appears Python supports [multiple inheritance](https://stackoverflow.com/questions/3277367/how-does-pythons-super-work-with-multiple-inheritance). You might be able to adjust the above code accordingly. – youngson Apr 29 '23 at 07:45
  • That just strikes me as a "has_a" relationship rather than a "is_a". The example simply has two classes that both _contain_ a Fish class. That doesn't allow the Fish class, for instance, to access the "fresh_water" property. – Mort May 08 '23 at 00:10
  • @Mort Yes, a fresh/salt-water fish is still a fish. The point I'm making is that is_a and has_a relational thinking can lead to inflexibility and dogma. I don't know anything about the API you are using but I see a lot of rigidity in the original code that could be solved by using composition and forgetting about classical inheritance. It is a widely-known Gang of Four principle to favor composition over inheritance and this is one reason for it. My code still functions identically. – youngson May 08 '23 at 01:24
  • @Mort As for `Fish` not knowing about the `fresh_water` property, that is an intentional decision because you can model the behavior for the different types of fish in their 'base' classes by implementing `IFish`. No need to do an if statement because the two classes are polymorphic so you can execute the same function and automatically do the different behaviors. Besides, the original code did not suggest to me that `Fish` needed to know that property, so this code functions under that assumption. – youngson May 08 '23 at 01:24
  • @Mort (Adding to my second comment) Furthermore, even with classical inheritance I don't see why a parent should ever know about its children. That is bad coupling. – youngson May 08 '23 at 01:35