PDA

View Full Version : Using accessors in Java constructors




(marc)
Dec 1, 2010, 03:52 AM
Hi,

I'm writing some Java code. In some of my setter methods, I do some error / value checking. So I decided to use these setter methods from within my constructor. But in Netbeans, I get a warning that it "might not be safe" to do so. I found this (http://www.java-forums.org/new-java/35082-setter-constructor.html) source trying to explain it like this:
If you don't make that class final and don't make the setName( ... ) method private or final someone else is able to extend your class and overrid the setName( ... ) method. Your constructor (in your base class) will call that method in the extending class instead of your implementation. Nobody knows what that method can do; hence the warning ... As a rule of thumb: a constructor shouldn't call methods that can be overriden.
and:
The method in the extending class will be called before the instance has had a chance to initialise itself. If the extending class's method depends on some initialisation your constructor will defeat that and possibly cause grief.

However, this doesn't quite sense to me. If I extend a class and then override some setter methods, I actually want that overridden setter method to be called, not my original one.

If this really isn't the way to do things, how else can I use the logic I have in my setter methods from within my constructor? Currently, I'm using a workaround of using a private (thus impossible to override) setter method which gets called by the public setter method and by my constructor. I guess this should solve the extending issues, am I correct?

Thanks!



robbieduncan
Dec 1, 2010, 04:43 AM
It could be an issue. Imagine we have a class like this:


public class BaseClass
{
private Date theDate

public BaseClass(Date aDate)
{
setDate(aDate);
if (theDate==null)
{
throw(new Exception("Invalid Date"));
}
}

public void setDate(Date aDate)
{
if (aDate.getYear()<2000) // getYear() is deprecated
{
// Do not set an invalid date: leave as the old one. Yes: this is bad practice, throw an exception instead!
return;
}
theDate=aDate;
}
}


This is sort of reasonable (although as noted in the comments not particularly good).

If we then had a subclass as below:

public class SubClass extends BaseClass
{
private int year;

public setDate(Date aDate)
{
year=aDate.getYear(); // This is deprecated
}
}


Now it is impossible to create an instance of SubClass as the internal private variable theDate of the superclass has not been set and the exception will be thrown. My personal preference for this sort of thing would be to do something like:


public class BaseClass
{
private Date theDate

private boolean isDateValid(Date aDate)
{
return (aDate.getYears()>=2000); // Still deprecated
}

public BaseClass(Date aDate)
{
if (!isValid(aDate))
{
throw(new Exception("Invalid Date"));
}
aDate=theDate
}

public setDate(Date aDate)
{
if (!isValid(aDate))
{
// Do not set an invalid date: leave as the old one. Yes: this is bad practice, throw an exception instead!
return;
}
theDate=aDate;
}
}


This will now allow everything to work as expected.

pilotError
Dec 1, 2010, 07:08 AM
Hi,
However, this doesn't quite sense to me. If I extend a class and then override some setter methods, I actually want that overridden setter method to be called, not my original one.


The problem comes when others are using your libraries. You have no idea who is going to do something 5 years down the road, or if your producing code for public consumption, it's better to avoid it.

Littleodie914
Dec 3, 2010, 09:00 AM
Now it is impossible to create an instance of SubClass as the internal private variable theDate of the superclass has not been set and the exception will be thrown.This isn't the case. The SubClass class is required to implement the superclass' constructor that takes a Date as a parameter. It is a compilation error otherwise.

The only way your SubClass class could exist (at compilation-time) is if you either added a default constructor to BaseClass, or implemented in SubClass:


public class SubClass extends BaseClass
{
private int year;

public SubClass(Date theDate) {
super(theDate);
}

public setDate(Date aDate)
{
year=aDate.getYear(); // This is deprecated
}
}

robbieduncan
Dec 3, 2010, 09:37 AM
This isn't the case. The SubClass class is required to implement the superclass' constructor that takes a Date as a parameter. It is a compilation error otherwise.

The only way your SubClass class could exist (at compilation-time) is if you either added a default constructor to BaseClass, or implemented in SubClass:



Yes, I was relying on that compiler behaviour. The constructor will be created that calls the super class constructor. This will call setDate. The setDate that will be executed should be (I believe) the one in the subclass.