0

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?

7
  • 3
    I 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 ... Sep 30, 2021 at 0:23
  • 2
    Beware 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. Sep 30, 2021 at 5:04
  • Doesn’t that break the SRP since my classes will have more than one reason to change?
    – human
    Sep 30, 2021 at 7:46
  • 1
    Nothing 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. 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
    Sep 30, 2021 at 12:26

2 Answers 2

3

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.

3
  • 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. 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. 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. Oct 1, 2021 at 16:17
0

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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.