36

For example, consider I have a class for other classes to extend:

public class LoginPage {
    public String userId;
    public String session;
    public boolean checkSessionValid() {
    }
}

and some subclasses:

public class HomePage extends LoginPage {

}

public class EditInfoPage extends LoginPage {

}

In fact, the subclass has not any methods to override, also I would not access the HomePage in generic way, i.e.: I would not do something like:

for (int i = 0; i < loginPages.length; i++) {
    loginPages[i].doSomething();
}

I just want to reuse the login page. But according to https://stackoverflow.com/a/53354, I should prefer composition here because I don't need the interface LoginPage, so I don't use inheritance here:

public class HomePage {
    public LoginPage loginPage;
}

public class EditInfoPage {
    public LoginPage loginPage;
}

but the problem comes here, at the new version, the code:

public LoginPage loginPage;

duplicates when a new class is added. And if LoginPage needs setter and getter, more codes need to be copied:

public LoginPage loginPage;

private LoginPage getLoginPage() {
    return this.loginPage;
}
private void setLoginPage(LoginPage loginPage) {
    this.loginPage = loginPage;
}

So my question is, is "composition over inheritance" violating "dry principle"?

  • 13
    Sometimes you need neither inheritance nor composition, which is to say that sometimes one class with a few objects does the job. – Erik Eidt Feb 12 '18 at 4:36
  • 23
    Why would you want all your pages to be the login page? Maybe just have a login page. – Mr Cochese Feb 12 '18 at 8:20
  • 29
    But, with inheritance, you have duplicate extends LoginPage all over the place. CHECK MATE! – el.pescado Feb 12 '18 at 16:12
  • 2
    If you have the problem in the last snippet a lot, you might be overusing getters and setters. They tend to violate encapsulation. – Brian McCutchon Feb 13 '18 at 6:31
  • 4
    So, make your page being able to be decorated and make your LoginPage a decorator. No more duplication, just a simple page = new LoginPage(new EditInfoPage()) and you are done. Or you use the open-closed-principle to make the authentication module that can be added to any page dynamically. There a lots of ways to deal with code duplication, mainly involving finding a new abstraction.LoginPage is likely a bad name, what you want to make sure is that the user is authenticated while browsing that page, while being redirected to the LoginPage or shown a proper error message if he is not. – Polygnome Feb 13 '18 at 12:34
46

Er wait you're concerned that repeating

public LoginPage loginPage;

in two places violates DRY? By that logic

int x;

can now only ever exist in one object in the entire code base. Bleh.

DRY is a good thing to keep in mind but come on. Besides

... extends LoginPage

is getting duplicated in your alternative so even being anal about DRY wont make sense of this.

Valid DRY concerns tend to focus on identical behavior needed in multiple places being defined in multiple places such that a need to change this behavior ends up requiring a change in multiple places. Make decisions in one place and you will only need to change them in one place. It doesn't mean only one object can ever hold a reference to your LoginPage.

DRY shouldn't be followed blindly. If you're duplicating because copy and paste is easier than thinking up a good method or class name then you're probably in the wrong.

But if you want to put the same code in a different place because that different place is subject to a different responsibility and is likely to need to change independently then it's probably wise to relax your enforcement of DRY and let this identical behavior have a different identity. It's the same kind of thinking that goes into forbidding magic numbers.

DRY isn't just about what the code looks like. It's about not spreading the details of an idea around with mindless repetition forcing maintainers to fix things using mindless repetition. It's when you try to tell yourself that the mindless repetition is just your convention that things are headed in a real bad way.

What I think you are really trying to complain about is called boilerplate code. Yes, using composition rather than inheritance demands boilerplate code. Nothing gets exposed for free, you have to write code that exposes it. With that boilerplate comes state flexibility, the ability to narrow the exposed interface, to give things different names that are appropriate for their level of abstraction, good ol indirection, and you're using what you're composed of from the outside, not the inside, so you're facing it's normal interface.

But yes, it's a lot of extra keyboard typing. As long as I can prevent the yo-yo problem of bouncing up and down an inheritance stack as I read the code it's worth it.

Now it's not that I refuse to ever use inheritance. One of my favorite uses is to give exceptions new names:

public class MyLoginPageWasNull extends NullPointerException{}
  • int x; can exist no more than zero times in a codebase – K. Alan Bates Feb 27 '18 at 12:37
  • @K.AlanBates excuse me? – candied_orange Feb 27 '18 at 12:59
  • ...i knew you were going to retort with the Point class lol. No one cares about the Point class. If I walk in to your codebase and see int a; int b; int x; I will push to have 'you' removed. – K. Alan Bates Feb 27 '18 at 15:18
  • @K.AlanBates Wow. it's sad that you think that that sort of behavior would make you a good coder. The best coder isn't the one that can find the most things about the others to bitch about. It's the one that makes the rest better. – candied_orange Feb 27 '18 at 15:27
  • Using meaningless names is a non-starter and likely makes the developer of that code a liability rather than an asset. – K. Alan Bates Feb 27 '18 at 16:24
129

A common misunderstanding with the DRY principle is that it is somehow related to not repeating lines of code. The DRY principle is "Every piece of knowledge must have a single, unambiguous, authoritative representation within a system". It's about knowledge, not code.

LoginPage knows about how to draw the page for logging in. If EditInfoPage knew how to do this then that would be a violation. Including LoginPage via composition is not in any way a violation of the DRY principle.

The DRY principle is perhaps the most misused principle in software engineering and it should always be thought of not as a principle for not duplicating code, but as a principle for not duplicating abstract domain knowledge. Actually, in many cases if you apply DRY correctly then you will be duplicating code, and this is not necessarily a bad thing.

  • 27
    "Single responsibility principle" is misused a lot more. "Dont repeat yourself" might easily come in as number two :-) – gnasher729 Feb 12 '18 at 7:31
  • 28
    "DRY" is a handy acronym for "Don't Repeat Yourself" which is a catch phrase, but not the name of the principle, the actual name of the principle is "Once And Only Once", which in turn is a catchy name for a principle that takes a couple of paragraphs to describe. The important thing is understanding the couple of paragraphs, not memorizing the three letters D, R, and Y. – Jörg W Mittag Feb 12 '18 at 7:53
  • 4
    @gnasher729 You know, I actually agree with you that SRP is probably misused a lot more. Although to be fair, in many cases I think they are often misused together. My theory is that programmers in general should not be trusted to wield acronyms with "easy names". – wasatz Feb 12 '18 at 12:31
  • 17
    IMHO this is a fine answer. However, to be fair: to my experience, the most frequent form of DRY violations is caused by copy-paste programming and just duplicating code. Someone told me once "whenever you are going to copy-paste some code, think twice if the duplicated part cannot be extracted into a common function" - which was IMHO very good advice. – Doc Brown Feb 12 '18 at 16:25
  • 9
    Abuse of copy-paste is certainly a driving force for violating DRY but simply forbidding copy-paste is poor guidance for following DRY. I would feel no remorse at all for having two methods with identical code when those methods represent different pieces of knowledge. They serve two different responsibilities. They just happen to have identical code today. They should be free to change independently. Yes, knowledge, not code. Well put. I wrote one of the other answers but I'll bow to this one. +1. – candied_orange Feb 13 '18 at 8:26
12

Short answer: yes, it does - to some minor, acceptable degree.

At a first glance, inheritance sometimes can save you some lines of code, since it has the effect of saying "my reusing class will contain all the public methods and attributes in a 1:1 manner". So if there is a list of 10 methods in a component, one does not have to repeat them in code in the inherited class. When in a composition scenario 9 of those 10 methods should be exposed publicly through a reusing component, then one has to write down 9 delegating calls, and let the remaining one out, there is no way around this.

Why is this tolerable? Look at the methods which are replicated in a composition scenario - these methods are exclusively delegating calls to the interface of the component, so containing no real logic.

The heart of the DRY principle is to avoid having two places in code where the same logical rules are encoded - because when these logical rules change, in non-DRY code it is easy to adapt one of those place and forget about the other one, which introduces an error.

But since delegating calls don't contain logic, these are normally not subject of such a change, so not causing a real problem when "prefering composition over inheritance". And even if the interface of the component changes, which might induce formal changes at all classes using the component, in a compiled language the compiler will tell us when we forgot to change one of the callers.

A note to your example: I don't know how your HomePage and your EditInfoPage look like, but if they have login functionality, and a HomePage (or an EditInfoPage) is a LoginPage, then inheritance might be the correct tool here. A less debatable example, where composition will be the better tool in a more obvious manner, would probably make things clearer.

Assuming neither HomePage nor EditInfoPage are a LoginPage, and one wants to reuse the latter, as you wrote, than it is pretty likely that one requires only some parts of the LoginPage, not everything. If that is the case, a better approach than using composition in the shown way might be to

  • extract the reusable part of LoginPage into a component of its own

  • reuse that component in HomePage and EditInfoPage the same way it is used now inside LoginPage

That way, it will typically become much clearer why and when composition over inheritance is the right approach.

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.