There are three ways I know of to deal with large container classes:
- large constructors
- post-constructor modifiers
None of them are 'nice'.
Your code is an example of the large constructor.
The post-constructor modifier system will have a bunch of 'set' methods, and you call them like:
The sub-cateogory option is not awful, it is done by grouping related parameters together. For example, the two times could be one class:
def __init__(self, created, modified):
self.created = created
self.modified = modified
then the file locations could be grouped as well
def __init__(self, path, public_url=None):
self.public_url = public_url
self.path = path
FileGroup would be, perhaps, the various 'types', like
Once you have sub-categorized your parameters, the constructor becomes simpler, like:
def __init__(self, name, locations, times, types, md5, size, public_key=None, preview=None):
But, though you have simplified this class's constructor, you now have to build up a hierarchy of data to initialize it.... which is not simple.
As for me, given the nature of your JSON data, it appears that the large constructor is the lesser of the evils. It is 'tailored' to match the JSON data format, and it works well.
My colleague says that the constructor is very large, and this code is horrible. Is he right?
The answer is: yes, it is large, and the code is ugly.... and he's right. But, not everything can be small, and pretty. This is the lesser of the evils.
I would not change it. Your class has a lot of related data, it has to get in there somehow.
Your constructor has the input
public_key but does not store that as a property.
You should consider using a generator in your
get_list_of_all_files: you should
yield each File, instead of returning the completed list.