3
\$\begingroup\$

I have a main-class that takes a list of sources and returns two objects for each source; one with the required data and one analytics tool.

The Analytics-class has different methods depending on what source it is. The Data-class extracts data from different paths and cleans the data in different ways depending on the source. Importing/exporting is made through pandas read_excel(). The analytics tool outputs some calculations based on what source the data comes from.

class Main_class(): def __init__(self, sources = ['a','b','c']): self.data_sources = {} self.analytics = {} for s in sources: self.data_sources[s] = Data(s) self.analytics[s] = Analytics(s, self.data_sources[s])

Right now, my solution is to have one Data-class and one Analytics-class which has if-statements to adapt the functionality of the class depending on the source. This is not a scalable or otherwise good solution, I basically have checks in both classes where I say

acceptable_sources = ['a', 'b', 'c'] if source not in acceptable_sources: raise ValueError(f"Only acceptable sources are: {acceptable_sources}")

Then, I need more checks to set the self.variables correctly, here's an example from the Data-class

self.data = {} if source == 'a': # if it's a, then there's 3 sources self.data[a] = pd.read_excel('a_1.xlsx') self.data[a] = pd.read_excel('a_2.xlsx') self.data[a] = pd.read_excel('a_2.xlsx') elif source == 'b': # if it's b, then there's 2 sources self.data[a] = pd.read_excel('b_1.xlsx') self.data[a] = pd.read_excel('b_2.xlsx')

This is problematic, since there will be a lot of if-statements as the number of sources increase, but it might be the best solution, I'm not sure. Using the same idea in my Analytics-class, there will be a lot of unused functions for each source-case. Let's say there are 30 functions for source a, 25 functions for source b and 40 functions for source c. Some of these functions might be shared across sources, and some will be unique. So whichever source I use, there will be a lot of unused methods which seems like a waste.

My first thought was to make Analytics and Data into abstract classes and create unique classes for each compatible source, but then I wouldn't be able to instantiate them in the for-loop in my main class. Then I thought that I could include them in a Class_holder which basically checks which class I want to instantiate, and if it exists to return an object of that class. So for example if I have X possible sources, the Class_holder would be able to handle and return X different classes, and if it doesn't exist return an error. It would look something like

from analytics import A, B, C # classes I should create with correct methods class Class_holder: def __init__(self, source, data): self.acceptable_sources = ['a', 'b', 'c'] if source in acceptable_sources: raise ValueError(f"Only acceptable sources are: {acceptable_sources}") self.source = source self.data = data def return_analytics_class(self): if self.source == 'a': return A(self.data) elif self.source == 'b': return B(self.data) elif self.source == 'c': return C(self.data)

And the classes I have called A, B, C could either be a combination of Data and Analytics, or I could separate it by having one Data and one Analytics-class for each source, then the Class_holder.return_class() would return a tuple with two classes. For the Class_holder-solution I would have to change my Main to something like

from a_file import Data from another_file import Class_holder class Main_class(): def __init__(self, sources = ['a','b','c']) self.data_sources = {} for s in sources: self.data_souces[s] = Data(s) self.analytics = {} for s in sources: self.analytics[s] = Class_holder(s, self.data_sources[s]).return_analytics_class()

But then I'm back to my original problem, where I need to have checks in both Data and Class_holder to see if the sources are compatible, however this might solve the problem of only instantiating the correct analytics-functions for each source.

It just doesn't feel like an optimal way of doing this kind of task, so I'm turning to codereview to ask for a bit of guidance, if you know any design pattern or other solution for this kind of problem, I would greatly appreciate it.

\$\endgroup\$

1 Answer 1

Reset to default
3
\$\begingroup\$

Presuming that you are using Python 3.6 ore newer, I would use abstract base classes and use the __init_subclass__() method to automatically register new subclasses and the sources they handle. A classmethod on the baseclass can then look up the appropriate subclass based on the source.

If the decision on which subclass to use is more complicated, each subclass can have a can_you_handle(self, source) method that returns True if is can handle the source. The base class from_source() method calls can_you_handle() on each subclass until it finds one that can handle the source.

Here's a base class:

class Data: registry = {} def __init__(self, source): self.source = source def __init_subclass__(cls, **kwargs): for source in cls.sources: if source not in Data.registry: Data.registry[source] = cls else: other_cls = Data.registry[source].__name__ message = f"class '{other_cls}' already registered for source '{source}'." raise ValueError(message) super().__init_subclass__(**kwargs) @classmethod def from_source(self, source): try: cls = Data.registry[source] return cls(source) except KeyError: raise ValueError(f"Unkown source: {source}") from None def __str__(self): return f"{type(self).__name__}('{self.source}')"

Some subclasses:

class Data_A(Data): sources = 'a','b' def method_for_source_A(self): print('doing something with Data_A') class Data_C(Data): sources = 'c','d','e' def method_for_source_C(self): print('doing something with Data_C')

Check the registry was populated:

print(Data.registry)

prints:

{'a': __main__.Data_A, 'b': __main__.Data_A, 'c': __main__.Data_C, 'd': __main__.Data_C, 'e': __main__.Data_C}

Try it out:

data = Data.from_source('b') print(f"{str(data)} using {data.source}") data = Data.from_source('e') print(f"{str(data)} using {data.source}") data = Data.from_source('q') print(f"{str(data)} using {data.source}")

prints:

Data_A('b') using b Data_C('e') using e --------------------------------------------------------------------------- ValueError Traceback (most recent call last) <ipython-input-82-072d12d66274> in <module> 5 print(f"{str(data)} using {data.source}") 6 ----> 7 data = Data.from_source('q') 8 print(f"{str(data)} using {data.source}") <ipython-input-70-bd84ab43455a> in from_source(self, source) 20 21 except KeyError: ---> 22 raise ValueError(f"Unkown source: {source}") from None 23 24 def __str__(self): ValueError: Unkown source: q

Try defining a subclass with a duplicated source:

class Data_F(Data): sources = 'f','e' def method_for_source_F(self): print('doing something with Data_F')

result:

--------------------------------------------------------------------------- ValueError Traceback (most recent call last) <ipython-input-124-39d1c8d7edf7> in <module> ----> 1 class Data_F(Data): 2 sources = 'f','e' 3 4 def method_for_source_F(self): 5 print('doing something with Data_F') <ipython-input-120-08031ccf6700> in __init_subclass__(cls, **kwargs) 14 other_cls = Data.registry[source].__name__ 15 message = f"class '{other_cls}' already registered for source '{source}'." ---> 16 raise ValueError(message) from None 17 18 super().__init_subclass__(**kwargs) ValueError: class 'Data_C' already registered for source 'e'.

class Analytics would be handled in an analogous manner.

Your main class would then look like this:

class Main_class(): def __init__(self, sources = ['a','b','c']): self.data_sources = {} self.analytics = {} for s in sources: data = Data.from_source(s) self.data_sources[s] = data analytic = Analytics.from_source(s) self.analytics[s] = analytic(data)

One more thing, __init_subclass__() takes keyword arguments. So you could write the code above like this (I just prefer using class variables as shown above):

 def __init_subclass__(cls, sources, **kwargs): # changed this line for source in sources: # and this one if source not in Data.registry: Data.registry[source] = cls else: other_cls = Data.registry[source].__name__ message = f"class '{other_cls}' already registered for source '{source}'." raise ValueError(message) super().__init_subclass__(**kwargs)

And then the subclasses would look like this:

class Data_A(Data, sources=('a','b')): # source in now a keyword argument def method_for_source_A(self): print('doing something with Data_A')
\$\endgroup\$
3
  • \$\begingroup\$ This is exactly what I was looking for, thank you so much for helping out! You've used a lot of things I haven't seen before, this answer helped me understand some of it. I'll definitely study and use this, and I'll probably look into using more inheritance between classes, for example if classes B,C,D has 3 shared functions, maybe they could inherit from class A which has these functions predefined (assuming that classes A,B,C,D are all subclasses to Data). \$\endgroup\$ Commented Nov 26, 2020 at 0:19
  • 1
    \$\begingroup\$ I was going to mention using "mixins" as a way to refactor common features from Data subclasses, but the answer was rather long already. \$\endgroup\$ Commented Nov 26, 2020 at 5:08
  • \$\begingroup\$ Thanks for mentioning it atleast, I hadn't heard of mixins before so I'll definitely check it out. And if I have any questions I'll create a new ticket for it. \$\endgroup\$ Commented Nov 26, 2020 at 8:48

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.