5

I know it is a bad (security) practice to call overridable methods from an object constructor in Java. However, for example, if the constructor has to initialize some data, it seems reasonable to call the respective setter method so that I don't copy code. The setters are public and not final. Is there any standard way of dealing with this, like declaring private setter methods, that the public ones call? To illustrate, here is some code:

class A {
    private double x,y;
    private privateSetX(double x1) { x=x1; }
    private privateSetY(double y1) { y=y1; }
    public A() { privateSetX(0); privateSetY(0); }
    public setX(double x1) { privateSetX(x1); }
    public setY(double y1) { privateSetY(y1); }
};
Improve this question
3
  • 3
    Presumably in this instance you wouldn't want to override the setters anyway? In which case they should be declared final. Commented May 23, 2011 at 23:41
  • Oli really hit this on the head. It begs the question of if you should even have public setters, but instead have only private setters and have public external-interacting behaviors (you know, when A class goes and "A-ifies things" or whatever it does) that call the setters. Commented May 23, 2011 at 23:50
  • 1
    @Oli Well, so it happens that this class is generic enough that I would want it to be possible for it to be subclassed and the methods - overridable, so I didn't make the setters final Commented May 23, 2011 at 23:51

3 Answers 3

Reset to default
2

If you really want to do this, create a secondary private setter method that is called by both the constructor and the public setter.

Improve this answer
Sign up to request clarification or add additional context in comments.

6 Comments

Oh, and one of the downsides of doing this is that you cannot have final instance fields.
@Oli: Yes, as in the given snippet. Just confirming. I don't recommend this as a matter of best practice, however.
@NathanRyan - It's hard to imagine a final instance field that would also have a setter. :)
@TedHopp That depends a bit on your coding standards. With a final instance field, the compiler is not able to determine the definite assignment, so the assignment statement in the setter would be flagged as a compiler error. But I've seen coding standards that require fields to be initialized via a setter in order to enable injection and/or instrumentation, despite the fact that these fields are final in practice. Whether these coding standards are appropriate is another matter :-)
There's a big difference between a final field (one declared with the keyword, which is what I was talking about) and a "final in practice" field. If a coding standard requires that a field be initialized via a setter, then the field simply cannot be declared final. Likewise (as you noted) there's just no getting around that the compiler won't allow a field declared final to also have a setter. Of course, with a builder pattern (as in @shoover's answer), one can have both: a final field that is initialized through a setter (which would then be in the builder class).
|
2

I think that initialising the data members directly in the constructor is better practice. If you call a method, then you have to go look at that method implementation to verify that it really is doing what it looks like it's doing. If you assign to a data member directly, you know that the initialisation is taking place. So in your code:

class A {
    private double x, y;
    public A() {
        x = 0;
        y = 0;
    }
    // ...
}

A constructor should usually be simple, deterministic, and obviously correct. Direct assignment satisfies these goals.

Improve this answer

4 Comments

Does not seem to be very DRY to me, either. Setters could do stuff like sanitizing input, etc. We would need to do the same in the constructor.
What's the aversion to duplicating a single assignment statement? The assignment in the constructor is initialisation, and the assignment in a setter method is changing an existing value. They're different purposes.
@K.Steff: Here's something that you may want to do instead, as a matter of better practice: Leave the individual assignments in the constructor and setters, and instead extract intermediate computation (like complex parameter validation) as separate, private methods.
@elusive Sanitize the input in package private static methods that have been unit tested and have no side effects. Call these methods from both the constructor and the accessor/mutators.
2

A better way to create an object that needs to have lots of different fields set during construction is to use the Builder Pattern.

Rather than duplicate the efforts of others, I will just point you to a most excellent SO answer on this topic.

If the problem is that you need to override setters during the constructor, you can create a hierarchy of Builders instead of, or in addition to, the hierarchy of the primary class that you're trying to build.

Improve this answer

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.