A very common C# pattern that breaks a very fundamental OOP principle
Asked Answered
M

6

14

Here is a very simple question, which I'm still very uneasy about:

Why is it widely accepted now for a class to return a reference to its private member through an accessor method? Doesn't this totally break the encapsulation principle? If this is OK, then why not just make the member public!?

public class EncapsulationViolator
{
  private object abuseMe;

  public object AbuseMe 
  {
    get { return abuseMe; }
  }
}

Edit The case I was considering is this

EncapsulationViolator ev = new EncapsulationViolator();

object o = ev.AbuseMe;

o.SetValue(newValue);

Now ev's state has change through transitivity because its member abuseMe's state has changed.

In the context of DDD, this is not OK if the object is an Aggregate Root. I quote

Allow external objects to hold references to the root only. Transient references to internal members can be passed out for use within a single operation only. Because the root controls access, it cannot be blindsided by changes to the internals.

[Domain-Driven Design, Eric Evans]

... setters schmetters ...

Modica answered 18/7, 2012 at 14:59 Comment(14)
There's no setter so there's not much room for abuseMultiplier
Sometimes people use this approach to do something before return is fired, that is basically a public field but where people need to do something before set/get. Also getters and setters are widely used in java too.Stimson
Brad: my point exactly. Once the caller has the reference after calling AbuseMe, they can now change abuseMe, right ?Modica
.NET serialization does not work on fields.Begley
@Samus, they cannot change it without a setter (set {abuseMe = value;})Nelson
Forte - Ok, then you have answered my question. Thanks.Modica
@ForteL., they can change the properties of the referenced object.Unmoving
They can't change the value of abuseMe if you don't have a setter, however if your object has some internal state that can be changed having the reference to this object, then yes, they can now change the internal state of the object. That's why you should remember this when returning some internal state to the caller.Eccrinology
@ForteL.: If the prop returns an object that implements IDisposable you could destroy the object.Begley
anderhil: Is the case you mentioned about altering the internal state through the returned reference's public member's considered a violation of encapsulation ?Modica
that's not a common pattern. you are abusing the language by telling that it is common at the moment.Kleinstein
ugurcode: r u saying that the above example, as is, is not a commonly used pattern to return a value copy of a reference ?Modica
I was just reading Evan's DDD book, and it sounds like if the object at hand is an Aggregate Root (which is and Entity), then no references to its members should be passed out (only a reference to the root is allowed). So in that sense, it sound's to be in violation : "Allow external objects to hold references to the root only. Transient references to internal members can be passed out for use within a single operation only. Because the root controls access, it cannot be blindsided by changes to the internals."Modica
ugurcode: so maybe this is what you meant. would you mind explaining yourself ?Modica
G
12

You're conflating the C++ term "reference" with the fact that C# passes objects by value (of their reference).

In this case the caller of the getter AbuseMe cannot swap out the private field abuseMe. Thus, there is no violation of encapsulation.

EncapsulationViolator x = new EncapsulationViolator();
object y = x.AbuseMe;
y = 17; // I have not changed x.AbuseMe

Debug.Assert(y != x.AbuseMe); // Passes!

Furthermore, property getters and setters allow for proper encapsulation of private fields and is functionally identical to implementing these as methods (in fact they are implemented as methods by the compiler).

One case in which returning a private variable could break encapsulation is when you return a reference to an array:

class X
{
    private int[] amazing = new int[10];

    public int[] Amazing { get { return this.amazing; } }
}

X a = new X();
int[] x = a.Amazing;
int[] y = a.Amazing;

x[2] = 9;
Debug.Assert(x[2] != y[2]); // Fails!
Glutton answered 18/7, 2012 at 15:10 Comment(8)
six: I guess the array case was the one I was considering while thinking of the violation, which you are saying is the exception to the rule, correct ? However, I guess I may have a fundamental misunderstanding of encapsulation then; when a reference is returned via an accessor (get{}/get()), and then the caller calls public methods on it, changing its state, which is now also changed in the called class b/c they share the reference, it is not considered a violation of encapsulation ?Modica
@SamusArin: well if your property is of type SomeType, would you really want to have to implement methods on OwningType to modify each and every single one of SomeTypes properties (and so on and so forth)? If you return a class reference in a getter, you then rely on proper "encapsulation" of that class.Glutton
six: Good point. You really helped clear up my concept of this fundamental oop property, thank you.Modica
You are mistaken. C# doesn't pass objects by reference. Object references are passed by value, unless you specifically use the ref or out keywords to pass the reference by reference.Azarcon
I meant "value of their reference"? I was avoiding using "pointer". Thanks!Glutton
Thanks Guffa... For anyone unclear about what Guffa and six are talking about, see msdn.microsoft.com/en-us/library/s6938f28.aspx it really helped me out.Modica
six: I'm not scared of pointers, and to express what Guffa is talking about in terms of them, he is saying that passing a Reference type (which is pointer/address/integer to the object on the managed heap) by value will pass a copy of the value of the address to the Reference type, and passing by ref(erence) will pass the address of this pointer (ie, a void * vs void **).Modica
Heh, thought I'd just point that out ;) I miss building custom data structures in c and c++.Modica
A
10

It depends on what kind of object the member is. If it for example is a string, then it's immutable, so you can't change the string.

If it is a mutable object, you can change the content of the object from outside the class, but you can't replace the object itself.

If the object should not be possible to change from outside the class, the getter should return an immutable version of the object.

The pattern could break the encapsulation if you do it wrong, but done correctly the encapsulation is intact.

Azarcon answered 18/7, 2012 at 15:8 Comment(2)
Guffa: By "If it is a mutable object, you can change the content of the object from outside the class, but you can't replace the object itself.", are you saying that you can modify the object's (ie, abuse Me) state (via its public methods), but cannot change where the reference points to ?Modica
Ok, your answer makes a lot more sense to me today and will help me out greatly. Thanks!Modica
B
8

I don't think it breaks encapsulation. The class still decides where the return value of AbuseMe comes from. It could come from a different member, or it could be recreated or copied every time.

Point is that the class decides what it allows users to do with that member (get/set or both and their visibility), it can perform validation and prevent invalid values to be set, and the users don't need to know where that value comes from.

Also, if you want to add custom logic to the get/set methods, you can do so without breaking compatibility with other assemblies.

Burny answered 18/7, 2012 at 15:4 Comment(5)
+1 for 'without breaking compatibility with other assemblies'Rely
Botz: all true, however I'm talking about this specific case, not about the utility of get/set accessors in general.Modica
@SamusArin In this specific case, i would say it's meant to enforce read-only access. Users can modify the object, but they can't change the object reference. If the class decides that's ok then it's no problem.Burny
Botz: ok, well I guess that is reasonable.Modica
Botz: now that the subtle misconception of encapsulation I was having has been cleared up, I see that this is a very well put and concise answer, thanks (I really wish I could accept multiple answers, b/c I definitly would accept this too... had to give it to sixlettervariables tho b/c he showed me the light on this).Modica
T
1

It's just syntactic sugar. It's not any different from Java's getXXX and setXXX methods.

Tubb answered 18/7, 2012 at 15:5 Comment(4)
... that still doesn't make it ok!Modica
It's not just sugar. There are differences between a field and a property. One difference is the way serialization treats fields vs. properties.Begley
@Boo It is syntactic sugar over creating get/set methods for fields.Chip
@AlexanderR: uhm, no. Again, you can't serialize a method. I understand how accessors are implemented as functions, but they are treated differently, and thus not just sugar.Begley
B
1

The point of the getter and setter is specifically to enforce encapsulation. The whole point is that you don't give access directly to the object, but force it to be accessed by a function that you define. Getters and setters ARE encapsulation. If you decide to just return the object, well that's your business but you aren't allowing direct access without hitting the getter.

Read this: http://en.wikipedia.org/wiki/Mutator_method

Brandling answered 18/7, 2012 at 15:11 Comment(2)
I see your point, and in general very true, however the specific example above does pass a "raw" reference, and I see this over and over, even in expert's examples.Modica
Giovanni - Good read, I always wondered about weather to use accessor methods within member functions, or to just directly access the private members (and about the performance differences). That article covers all that stuff, thanks!Modica
C
1

IMO - too many answers here are promoting getters/setters. Getters/setters are great for procedural code, you either do some calculation and set the result or grab the value(s) and make some decision.

A well known principle in OO programming is Tell don't ask which basically says you shouldn't ask an object of its internal state to make a decision.

That being said, I use accessors/properties myself. However, when possible, I try to avoid them.

Copenhagen answered 21/7, 2012 at 18:36 Comment(1)
Roger: Interesting thought about the "tell don't ask" statement. Need to think about that one for a bit (though my inital thought that comes to mind is "side-effects" - you base your next decision based on a previous result, which seems to be in contrast to what your proposing here). Thanks for the insight.Modica

© 2022 - 2024 — McMap. All rights reserved.