• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

overridable method call in constructor

 
Bartender
Posts: 1810
28
jQuery Netbeans IDE Eclipse IDE Firefox Browser MySQL Database Chrome Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I found this post but I still don't understand. Can someone Explain Like I'm Five?

This code is for a custom tag that creates a select/option element. Why is the IDE/compiler complaining about the "this" references with the error "overridable method call in constructor"?

[edit] not sure if it matters, but MachineSelect extends a custom class called SelectBox. That's where "addItem" and "defaultValue" come from.
 
Rancher
Posts: 4801
50
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
It is generally not advisable to call methods that can be altered by a subclass in your constructor, as you will have no idea what that subclass might do. It can make things a bit fragile.

You can get around this by making the method final.

You could also make your class final, which would (should?) prevent the warning.
 
Sheriff
Posts: 11604
178
Hibernate jQuery Eclipse IDE Spring MySQL Database AngularJS Tomcat Server Chrome Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Because it's considered to be a bad practice with potential risk to introduce hard to spot bugs. Here you'll find an example of such a bug... Although this one is not hard to spot as it's only 35 lines of code
 
Marshal
Posts: 79151
377
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
There are two possible problems you can get in constructors (and lots of others).
There is letting a this reference escape, which permits outside code to access this object before its initialisation is complete. Or while it is in an inconsistent state. I saw an example here on the Ranch in a Pupil class a long time ago. The constructor took a School reference as a parameter and included something likeNow, the register call might be expedited and there is no guarantee that the Pupil object's fields will all be fully initialised before then. Particularly nasty in a multi‑threaded environment, if the school's methods are called at the same time.

The other thing is calling a non‑private non‑final method. I think this is what the IDE is noticing here rather than the preceding problem. Line 11: AddItem?? If you override that method either in this class or in a subclass, you may suffer unpredictable behaviour when you call that constructor. So you should mark all methods called from the constructor final (or private: I don't think you need to say both).
 
Sheriff
Posts: 3837
66
Netbeans IDE Oracle Firefox Browser
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
When you call an overridable method from a constructor, there is a risk that a child class will override that method and access attributes it declares. When the constructor executes, the overriden method is called, but the attributes declared in the child class aren't initialized yet. Even its constructor hasn't been called finished yet (edit: it has been called, but it called parent class constructor as its first action and that call hasn't returned yet).

If the child method doesn't use any attributes of the child class, it's safe. If it accesses them, strange things can happen - the class could "see" a final attribute change its value, for example.

(As far as I know, C++ "solves" this issue by calling the original, not overriden, method from constructor. Hard to say which solution is better.)
 
Roel De Nijs
Sheriff
Posts: 11604
178
Hibernate jQuery Eclipse IDE Spring MySQL Database AngularJS Tomcat Server Chrome Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Dave Tolls wrote:You could also make your class final, which would (should?) prevent the warning.


Then you'll get a compiler error instead (if you have subclasses of course which seems to be the case based on the OP)
 
J. Kevin Robbins
Bartender
Posts: 1810
28
jQuery Netbeans IDE Eclipse IDE Firefox Browser MySQL Database Chrome Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Wow, thanks for all the great answers. I think it's sinking in now. I'll ponder this while I get some lunch.
 
Dave Tolls
Rancher
Posts: 4801
50
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Roel De Nijs wrote:

Dave Tolls wrote:You could also make your class final, which would (should?) prevent the warning.


Then you'll get a compiler error instead (if you have subclasses of course which seems to be the case based on the OP)



In that case, yes, but you get that warning whether you have subclasses or not.
 
Bartender
Posts: 732
10
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
An example of this problem is in many Swing classes - if you create a subclass of JPanel, for example, and override setBackground() or setFont(), your overridden methods may be called before the body of your constructor gets executed. Then problems occur if your overridden methods reference any of your new instance fields.
 
J. Kevin Robbins
Bartender
Posts: 1810
28
jQuery Netbeans IDE Eclipse IDE Firefox Browser MySQL Database Chrome Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Okay, follow-up dumb question. I don't want to modify the SuperClass (SelectBox). It's used all over and I don't know what sort of hell might break loose.

So, with that in mind, what would be a better/correct way to write this code?
 
Fred Kleinschmidt
Bartender
Posts: 732
10
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
For any methods called in your constructor that are overrides or public, create a private method to do that functionality, and call hose private methoids from your constructor and from the public or overridden methods. For example, let's take AddItem:

 
Saloon Keeper
Posts: 15484
363
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
If you make MachineSelect final, the methods won't be overridable and your problem is solved.
 
Fred Kleinschmidt
Bartender
Posts: 732
10
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
But your superclass might call some method if its own that you have overridden. And it is quite often unfeasible to make all of your classes final.
 
Stephan van Hulst
Saloon Keeper
Posts: 15484
363
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Fred Kleinschmidt wrote:But your superclass might call some method if its own that you have overridden.


That's the superclass' problem. They shouldn't call overridable methods in the first place.
 
J. Kevin Robbins
Bartender
Posts: 1810
28
jQuery Netbeans IDE Eclipse IDE Firefox Browser MySQL Database Chrome Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Stephan van Hulst wrote:If you make MachineSelect final, the methods won't be overridable and your problem is solved.


No joy. Still get the compiler warnings.
 
J. Kevin Robbins
Bartender
Posts: 1810
28
jQuery Netbeans IDE Eclipse IDE Firefox Browser MySQL Database Chrome Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Fred Kleinschmidt wrote:For any methods called in your constructor that are overrides or public, create a private method to do that functionality, and call hose private methoids from your constructor and from the public or overridden methods. For example, let's take AddItem:


<code deleted>
Thanks! This looks like a workable solution. I'll try this right now.
 
J. Kevin Robbins
Bartender
Posts: 1810
28
jQuery Netbeans IDE Eclipse IDE Firefox Browser MySQL Database Chrome Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
That did it! No warnings, the code works the same, and I feel better about having more robust code in the project.

Many thanks to everyone!
 
Stephan van Hulst
Saloon Keeper
Posts: 15484
363
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

J. Kevin Robbins wrote:

Stephan van Hulst wrote:If you make MachineSelect final, the methods won't be overridable and your problem is solved.


No joy. Still get the compiler warnings.



These are IDE warnings. You might want to report this as a bug?
 
Martin Vashko
Sheriff
Posts: 3837
66
Netbeans IDE Oracle Firefox Browser
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

J. Kevin Robbins wrote:That did it! No warnings, the code works the same, and I feel better about having more robust code in the project.


Depending on exact circumstances, that might not actually fix the issue. If you call an overridable method from your private method (edit: I assume you do; you need to call AddItem inherited from the parent class), you don't get the warning, but still can get the wrong behavior. Consider:

This code produces the following output:

Grandchild, value=0
Grandchild, value=5


We can see the value of a final attribute has changed. This is one of the ways this problem manifests itself.

Note: in my IDE, I don't get any warning on this code, but I don't get any even when I call method directly in the constructor - don't know why. Static code checkers, such as FindBugs, should be able to identify this issue in both forms, though.

In my opinion, there are a few options of handling this case:

1) Do nothing. Just document methods that are called from constructors, so that developers are aware what's happening. That's obviously risky, especially if some developers on the team aren't well acquainted with the ramifications of this arrangement.

2) Make the method final in the class that calls the method in its constructor. That is a good solution if the child classes of the problematic class do not need to override the method. It might work for you, if I understand your case well. The code would look like this:

3) Make your entire Child class final. This obviously fixes the issue, but you can't create subclasses.

4) If you don't override the method in your class, you can call the super version of the method in constructor:
Caveat: in your case, someone might want to create a child from your class and override AddItem to do some additional action. He or she will wonder why his/her overriden method wasn't called when you were adding the items in your constructor calls.

5) Move the logic from your constructor elsewhere. You could create a static method that will create the instance and then call the overridable methods from the outside of the constructor. You don't introduce unwanted side effects, the class is still inheritable and the method isn't final. Applied to your case it might look like this:
This is probably the cleanest solution, if you can't make your class final. The only downside is that the logic that was originally in the constructor isn't inheritable now.

Note: this code mixes JDBC code and UI creation, and I think it isn't a good idea. I'd suggest separating these two pieces of code, having a DAO class that would read from the database into some data class (perhaps just a list of POJO items) and use that list to construct the GUI controls. But this is unrelated to the issue you've asked about.
 
Fred Kleinschmidt
Bartender
Posts: 732
10
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
NO - you NEVER call the overridden method from the private method. It's the other way around. The overridden method calls the private method, and the private method does all the work.
 
Martin Vashko
Sheriff
Posts: 3837
66
Netbeans IDE Oracle Firefox Browser
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Fred Kleinschmidt wrote:NO - you NEVER call the overridden method from the private method. It's the other way around. The overridden method calls the private method, and the private method does all the work.


You're right, that's a good approach.

However, if I have understood this particular case well, the method which needs to be called from the constructor is the AddItem method, which is inherited from the parent class. That approach therefore can't be employed here directly, some more refactoring of the class hierarchy would be needed.
 
Fred Kleinschmidt
Bartender
Posts: 732
10
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
No, the constructor calls the private method, and the public method(AddItem) also calls the private method (That is the ONLY line of code in the public method).
 
Stephan van Hulst
Saloon Keeper
Posts: 15484
363
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
The problem in this particular case is that addItem is a member of the superclass, so you can't control what it does.
 
Fred Kleinschmidt
Bartender
Posts: 732
10
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
But my addItem() overrides the superclass addItem(),so the superclass method never gets invoked (unless I call super.addItem())
 
Martin Vashko
Sheriff
Posts: 3837
66
Netbeans IDE Oracle Firefox Browser
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
It can be made to work and even re-use the inherited functionality:

This produces:
Parent
Child
Instance constructed
Parent
Child
Grandchild, value=5


It still has a small disadvantage that the (indirect) method call in the constructor doesn't invoke Grandchild's implementation. Of course, we did it precisely to avoid Grandchild's method from being called this way, but it's not immediately obvious.

Let's say someone would want to monitor item creation in the OP's class. He could extend MachineSelect, override AddItem and add logging to the overriden method. However, he would then observe items being created by the class constructor, and no trace about them in the log. In other words, it's a solution which has some non-obvious side effects. In a really, really complicated projects, it could give some headaches.
 
Fred Kleinschmidt
Bartender
Posts: 732
10
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
As I said, the ONLY statement in the overridden method should be one that calls the private method. The logging functionality would then be added in the private method. The private method could even call super.addItem() if desired (for example, if it wants to do whatever its superclass does, plus add logging).
 
Martin Vashko
Sheriff
Posts: 3837
66
Netbeans IDE Oracle Firefox Browser
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Fred Kleinschmidt wrote:The private method could even call super.addItem() if desired (for example, if it wants to do whatever its superclass does, plus add logging).


In this case, any extended functionality (logging) has to be put into the same class. It wouldn't work in a subclass. So, if we have to modify the MachineSelect class anyway, we can make the method or the entire class final - it will fix the issue and it will be transparent.

In almost all cases, your solution is very good. I'm not saying that it would be a problem for OP's case. My logging example is artificial; I just wanted to point out that it makes the call from the constructor non-overridable, and it can lead to unexpected behavior, especially in complicated class hierarchies.
 
Ranch Hand
Posts: 234
12
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

you NEVER call the overridden method from the private method


I know that an overridden method should never be called from a constructor but I've never heard that an overridden method should never be called from a private method. Is this a general rule? If so, why should you never call an overridden method from a private method?
 
Fred Kleinschmidt
Bartender
Posts: 732
10
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
No - this thread was about overridden (or overridable) methods called from a constructor, and replacing that call with a call to a private method that performs the task of that public or overridden method, and having the overridden method also call that private method. That way a subclass' overriding method can never be invoked by the superclass constructor before the body of the subclass constructor is executed.
 
J. Kevin Robbins
Bartender
Posts: 1810
28
jQuery Netbeans IDE Eclipse IDE Firefox Browser MySQL Database Chrome Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Wow, this blew up. I'm struggling to keep up, but this is very interesting. Let's see if I've totally screwed this up.

The MachineSelect class as it is now:

You're thinking "what the heck is Option?". It's a class within the SelectBox class. (I know this part is cringe-worthy; not my code.)

From what I've read here, I think I better make my class final. We have other classes that extend SelectBox, so if someone comes along and needs a different select control tag they can extend SelectBox and start from there.

So if I declare MachineSelect as final am I good, or have I created a time bomb that's going to get me one of these days?
 
Fred Kleinschmidt
Bartender
Posts: 732
10
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Your constructor is still calling this.AddItem - it should call this.MyAddItem directly.
 
J. Kevin Robbins
Bartender
Posts: 1810
28
jQuery Netbeans IDE Eclipse IDE Firefox Browser MySQL Database Chrome Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Thank you for catching that. It's been corrected.
 
Martin Vashko
Sheriff
Posts: 3837
66
Netbeans IDE Oracle Firefox Browser
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
The solution you've posted seems okay (assuming you fix the issue pointed out by Fred!) - you don't call overridable method of your own class from your MyAddItem method. The disadvantage of this solution is that you duplicate code of the original AddItem method. If the original AddItem changes for any reason, you (or someone else) must not forget to update MyAddItem as well. The chances of updating that method are slim, but this kind of code duplication is very unfortunate.

My advice would be:
  • If you can't foresee the MachineSelect having any children, make it final. Document why it is final.
  • If you think it could have a subclass sometime, you could still make the AddItem method final inside the MachineSelect class (and document why is it made final).
  • Other than that, the solution #5 I've posted yesterday avoids both duplicate code and unexpected side effects.

  • None of these solutions creates a time bomb of any sort (at least as far as calling overridable methods from constructor goes).
     
    Fred Kleinschmidt
    Bartender
    Posts: 732
    10
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator

    If the original AddItem changes for any reason, you (or someone else) must not forget to update MyAddItem


    That's why when I do this I usually write the public method as:
    reply
      Bookmark Topic Watch Topic
    • New Topic