I am looking for a suitable design which uses composition to allow three classes to share some logic.
The problem I am solving is that I have to read 3 different json configuration files from my application, and each file needs to have a different class that allows to filter specific data accordingly, however I need to share the logic of reading the files from the filesystem and validating the schema and format. I am looking to avoid inheritance and use composition instead, and I am trying to avoid repeating code as much as possible.
The three classes I currently have are:
CompanyConfig
getCompanies()
getCompany()
RoleConfig
getRoles()
getRole()
getRolesBy()
ThemeConfig
getThemes()
isThemeEnabled()
On each of these classes constructors I am using a class LoadFileContents and ValidateFileContents, however the logic to invoke these two classes repeats on each of the config classes.
Any ideas?
-
3I don't see a point to factoring out the logic around invoking the two utility classes. I can't imagine that they take more than a couple of lines of code each. Repeating 6 lines of code across three different classes is not a crime. Yet ...BobDalgleish– BobDalgleish09/30/2021 00:23:06Commented Sep 30, 2021 at 0:23
-
2Beware of the DRY police, they're after you! Seriously, wanting to avoid using a config file loader in separate places where configurations are needed is like wanting to avoid addition since you already used it in another function. It just doesn't make sense.Hans-Martin Mosner– Hans-Martin Mosner09/30/2021 05:04:56Commented Sep 30, 2021 at 5:04
-
Doesn’t that break the SRP since my classes will have more than one reason to change?human– human09/30/2021 07:46:00Commented Sep 30, 2021 at 7:46
-
1Nothing wrong with violating design principles really; the main factor in maintainability and code quality is automated test coverage rather than the structure of the code itself; that is to say, if you ever need to revisit the code later on in future then the extent of your test coverage will primarily determine how easy or difficult it is for somebody to understand it, reason over it and change it.Ben Cottrell– Ben Cottrell09/30/2021 11:24:05Commented Sep 30, 2021 at 11:24
-
"the logic to invoke these two classes repeats on each of the config classes" - could you give us an example of this logic, to give us an impression if it is really worth the effort of making the code DRY here?Doc Brown– Doc Brown09/30/2021 12:26:40Commented Sep 30, 2021 at 12:26
2 Answers 2
There is no design pattern for this. There are as many ways to do this as there are developers on this planet, so go for the most obvious design first.
In this case, you have 3 classes that read data from a file, validate the file format, and return strongly typed objects. Inheritance is an option here. The guideline is "favor composition over inheritance" not "don't use inheritance." The litmus test for whether or not inheritance is an appropriate solution is the "is a" test:
ChildClass is a ParentClass
If you insert your own class names into the sentence above, and the sentence is still logically true, then inheritance might be a valid solution. Having an abstract Config class with one derived type for roles, companies and themes passes this test:
CompanyConfig is a Config
RoleConfig is a Config
ThemeConfig is a Config
All three sentences make logical sense. Put the common logic in an abstract parent class.
-
Config
is not a thing if it doesn't have behavior on its own. Inheritance is not for sharing "utility" methods among subclasses. The OP is right to go for composition here. Also there should never be inheritance if there is no need for polymorphism.Robert Bräutigam– Robert Bräutigam10/01/2021 15:50:03Commented Oct 1, 2021 at 15:50 -
@RobertBräutigam: I mostly agree with what you are saying. Within the realm of configuration, though, I'm just not sure if being this picky about inheritance is worth it. I do disagree that inheritance is only for polymorphism. It is also valid to use inheritance for specialization, which I consider to be related to, but different from polymorphism. Sub types can extend or build on the behavior of the parent type, even if no abstract members are present in the parent type. I was coming from this angle in my answer.Greg Burghardt– Greg Burghardt10/01/2021 16:14:47Commented Oct 1, 2021 at 16:14
-
I must strongly agree with "Inheritance is not for sharing "utility" methods among subclasses", though, if sharing utility methods is the only reason for the parent class.Greg Burghardt– Greg Burghardt10/01/2021 16:17:02Commented Oct 1, 2021 at 16:17
I agree with @Greg above.
As an additional point, consider moving the common functionality of I/O and validation out to separate classes.
Further, utilize abstract base classes for them and then compose them into the config classes (if a reader and validator makes sense to be a component of a config in your model).
This will help you attain several aspects of SOLID design, the key aspects being Single Responsibility and Dependency Inversion.
The advantages of such a split are:
SRP and changes across the classes will not pollute one another (your colleagues will thank you during reviews).
Config classes will focus on hierarchy and logic for transforming configs (business and product can keep making changes to this class, allow you to focus on more testing for this)
If composing the I/O and validation classes into the config classes, you can sub it out for more specific implementations of them (utilize O and L of SOLID - Open and closed principle and Liskov Subsitution Principle) (This will allow existing classes using old readers to avoid any changes)
Hope that helps.
Explore related questions
See similar questions with these tags.