Is it good practice override methods with a higher visibility?

Question

Answering this question: How to GUI - Using paintcomponent() to initialize a GUI and then to add GUI based on mouse I've stated this:

You don't override paintComponent() properly. This is a protected method, not public. If you add @Override annotation on this method then the compiler will complain.

But @peeskillet wisely pointed out this:

The compiler will not complain about public or protected with paintComponent. You can override with a higher visibility but not a lower one. public is higher than protected so there's no problem.

Which is certainly true. But now it arises this question: Is it good practice override with a higher visibility?

Addendum

Link to JComponent.paintComponent() javadoc.

Image of Netbeans not complaining at all:

Tupi

If you need it, yes. Otherwise, don't do it. – Cystectomy

wow. I'm stumped. Does java really let you do that? this really deserves a post in Coding Horror... – Wally

Answer 1

One reason to do this is if you have a method you need to override elsewhere in your project and the current scope does not allow you to. Typically when using default instead of protected.

By creating a new sub class in the right package elsewhere in your project with revised scope, you can then create an anonymous class in your code where you need it as you are now allowed to override the troublesome method instead of having to do with reflection (which makes for quite unreadable code).

This is also a reason for library classes never to be final because then you cannot do things like this.

---

EDIT: I was asked to elaborate on why this is relevant for Dependency Injection. I can only speak based on my own experiences, first with Guice and now with Dagger.

Dagger uses Constructor Injection which basically mean that a class will get all its dependencies provided as arguments to the constructor (and only there) and that the glue code binding this together is listed in a Dagger @Module. In this module it is usually very convenient to return a subclass of a library class augmenting with log statements or providing a custom toString() method. In order to actually DO this without reflection tricks the clasess cannot be final and you need to be able to override methods and to use fields in the superclass directly. Hence no final classes and both types need to be at least protected instead of private!

(and I can strongly recommend using Dagger because it moves the dependency resolving in the java compiler giving the IDE the information needed to help you resolve problems at compile time, instead of depending on magic in the runtime in the wild. I am still amazed at the insight in the Java ecosystem the Dagger designers had to even get this idea and then realize it)

Guthry

+1. That's a good reason to do so. You have mentioned in a comment something about dependency injection. Could you please add something about it in your answer? – Tupi

Awarded bounty to this question because: 1 - the OP liked it and 2 - it mentions "anonymous inner classes" which is essentially a workaround to java's limitations (lack of delegates and events and real generics). – Wally

Anonymous classes was the compromise decided upon in the original Java specification. Full lambda support was found too potentially confusing to the C++ programmers they wanted to reach. The generics system is deliberately crippled in order to be fully backwards compatible with existing binary code which was paramount in Sun based on their Solaris mindset. – Bochum

@ThorbjørnRavnAndersen thank you very much for the edit. It deserves more upvotes but unfortunately I can only vote once. Think the bounty is well awarded. – Tupi

Answer 2

From Java Tutorial, Controlling Access to Members of a Class:

If other programmers use your class, you want to ensure that errors from misuse cannot happen. Access levels can help you do this.

• Use the most restrictive access level that makes sense for a particular member. Use private unless you have a good reason not to.
• Avoid public fields except for constants. (Many of the examples in the tutorial use public fields. This may help to illustrate some
  points concisely, but is not recommended for production code.) Public fields tend to link you to a particular implementation and limit your flexibility in changing your code.

This advice is aimed to reduce coupling:

To achieve the best encapsulation (information hiding) you should always declare methods with the least visibility that works. In small programs there's really no problem, but in large programs this problem of excessive coupling is very serious. Coupling occurs when one part depends on the specific implementation of another. The more coupling there is the more costly it becomes to make changes because too much code depends on specific implementations. This causes software rot - a program becomes progressively less usable because it can't be easily upgraded.

So, increasing visibility is actually not a good idea. Such code could couse trouble in future development and maintenance.

Gallenz

+1, though my real point is why does the java compiler allow that, and I'm looking for an official source (somebody from the java design team or the like (if there is such thing) – Wally

Compiler detects only compilation errors. These are architectural concerns, beyond language syntax. This ability should be used with caution, but not prohibited, because sometimes it's useful. – Gallenz

the C# compiler detects that, though. – Wally

I disagree. I have found that - especially with dependency injection - you need to be able to subclass a given class with ease, and access the fields easily. – Bochum

@ThorbjørnRavnAndersen: Instead of subclassing, you should be extracting the interface of the class you want to replace, changing all your usages of the concrete class to use the interface instead, and then making a replacement that implements that interface. This means you can later remove the replaced class completely if you want, because the new implementation doesn't depend on it. – Whimsy

Thank you for the answer, you have +1 from me. However I've accepted Thorbjørn Ravn Andersen's answer because he has mentioned a really good reason to override with higher visibility (anonymous class). – Tupi

@LucasWilson-Richter that depends on whether you are in full control of that code or not. Also "should" is a very strong term - what if all I need is a revised toString on an existing class? – Bochum

@ThorbjørnRavnAndersen A strong term for a strongly-held opinion :) And you're absolutely right that my ahem suggestion depends on you having control of the source code. However, even if you don't have control over the code there are often more flexible alternatives to inheritance. – Whimsy

Answer 3

If you NEED to access the method from outside the class/subclass it resides in, then a solution is to override the visibility with the public parameter. Best practice is to have your variables and methods on the lowest visibility possible.

Ease

Answer 4

You can advance a method's visibility to become more visible but not less visible, and so paintComponent can be overridden and declared as a public method. Having said that, I'll add that you shouldn't do this. When overriding the method, you should instead keep the visibility unchanged unless you have a very good reason to make it more visible.

Thermy

+1 for When overriding the method, you should instead keep the visibility unchanged unless you have a very good reason to make it more visible. Totally agreed, but what would be a good reason? I can't think in any common reason and we can always extend, make a public method with another different name and do the call to protected method inside this public method. This way the protected method won't be exposed directly. – Tupi

Answer 5

From the OOP point of view there is no problem with it. By extending a class you can do the following things:

• change some of the functionality (by overriding a method)
• extend the class' interface (by adding a new public method)

When you override a method and change its visibility you are doing both: You - obviously - change the functionality but also extend the interface. From the class' client's perspective you are actually creating a new method in the class' interface. This new method coincidentally has the same name as some internal method in the superclass, but the client doesn't care (or doesn't even know about this). So why should you not?

On the other hand there is the question "why do you need to do this?". The superclass' author probably had some thoughts about the visibility of this method and found that its functionality is not meant for the outside world. I don't mean that it's wrong to do so but you have to question your motives to increase the visibility, because it might be a hint for probably bad design in either your or the superclass' code.

Btw: As pointed out here disallowing this language feature can even be harmful.

Oto

Thank you for the answer, I like the explanation. As I've said we can add a public method and call the superclass' protected method inside of this making it transparent to whoever that calls the public method. I think because of this then doesn't make much sense forbid increasing visibility. However IMO these are two different things: the first one extends the class interface but the second one deliberately exposes a superclass' method to the outside. Don't feel this is right. – Tupi

Answer 6

Overridden methods, just like all other methods, should only be declared public if you want external code to be able to call them (unless you have no choice because the overridden method is declared public). In your case the overridden method paintComponent should only be called by Swing, thus keeping it protected is the best option.

Plumbism

Even if you as the designer have an opinion on what I should call, I might have a need to use your tool for something you did not envision and therefore did not allow for. An example - I used a java based FTP server (as it allowed for having a memory based file system) for an integration test. In order to see that sessions were correctly terminated I needed to access the state of the sessions which was not allowed and impossible to do without reflection. – Bochum

Answer 7

To add to what Francois was saying, the OOP principle at play here is the "Open Close Principle," which says you should be able to extend, but not modify an object. 'Raising' the visibility of a method by overriding it is simply extension as Francois pointed out.

Pretender

Answer 8

There are many arguments to be made for changing the visibility. But just think about the fact that you would be changing the interface (in the sense of API) of that class. Read up on evolving APIs and associate that information to your question.

So in my book good practise would be to not change the visibility - if at all possible. Otherwise you have to carefully consider the consequences - if only in the long run.

Pinchas