Open/Closed principle and violation of encapsulation

Question

Could you please check if the following code is correct or not? The fact is that I found something similar in propduction code and I have doubt if it matches Open/Closed principle.

public abstract class CustomClass {

    private ClassThatSetEnvironmentProperty sysProp = new ClassThatSetEnvironmentProperty("SYS_PROPETY", "SYS_PROPERTY_VALUE");

    // some code here

    void setSysProp(ClassThatSetEnvironmentProperty sysProp) {
        this.sysProp = sysProp;
    }
}

My understanding is the setter is defined for unit-tests possibilities only (to mock ClassThatSetEnvironmentProperty). But in this case the setter allows concrete inheritants to change defined state. From my perspective it violates encapsulation. More over I think that it is also violates open/closed prinicple. Frankly, some of my coleagues takes an opposite view. I really have not to much experience so it is hard for me to recognise it. Please share your opinion here. Thank you.

You are right and I totally agree with you. But what about current case? — aime, Sep 21 '16 at 17:52
The definition of the Open/Closed principle states that one should be able to change the behaviour of a class without modifying its source code. In this case, I don't think it violates the principle at all. The source code stays the same but `sysProp` is changed. — christopher, Sep 21 '16 at 17:56
Current case uses setter injection; I would use constructor. You could make it immutable and not break encapsulation. — duffymo, Sep 21 '16 at 17:56
@christopher Ok, thank you. But my understanding is `ClassThatSetEnvironmentProperty` instantiation is defined in abstract class to defined a part of common state for inheritants. If the property can be changed would it be better to define it in each concrete class? — aime, Sep 21 '16 at 18:05
If each concrete class is independent, then yes. Otherwise no. You added it to your abstract class. — duffymo, Sep 21 '16 at 18:14

score 5 · Answer 1 · answered Sep 21 '16 at 17:57

5

This doesn't directly relate to the Open Closed Principle, The Open Closed Principle just means that to add new behavior to your system you should create a new implementing class rather than change old ones. Using an abstract class for that is fine.

The one thing that does violate encapsulation (which is a different principle) is the package-accessible dependency setter. You can fix that issue by changing it to be a protected setter. Then extending classes can set their own, but outside callers cannot change the states of your objects.

protected final void setSysProp(ClassThatSetEnvironmentProperty sysProp) {
    this.sysProp = sysProp;
}

answered Sep 21 '16 at 17:57

Silas Reinagel

4,155
1
21
28

I agree. Thank you – aime Sep 21 '16 at 18:07
Protected is *less* restrictive than package private (default). – Ole V.V. Sep 21 '16 at 18:12
@OleV.V. They are differently restrictive. Protected ensures that all extending classes have access to it. For shared APIs, that is a critical requirement. – Silas Reinagel Sep 21 '16 at 18:29
1

@SilasReinagel, either you or I have not completely understood [Controlling Access to Members of a Class](https://docs.oracle.com/javase/tutorial/java/javaOO/accesscontrol.html). – Ole V.V. Sep 21 '16 at 19:01

Open/Closed principle and violation of encapsulation

1 Answers1