I see several examples of code like "project.Resources.Assign(resourceId);" in the C# BO book sample ProjectTracker.
The "Resources" property getter is returning a reference to the property's private data so the UI code can call the Assign method on it. Iit is not making a copy and returning a copy of the private data.
Doing this breaks encapsulation (by returning a reference to the private instance member).
Any suggestions on how to do this better - avoid violating encapsulation and yet provide the desired behavior?
Thanks.
I'm not sure I understand your question.
project.Resources returns a reference to a business object. "project" itself is a reference to a business object as well - the two objects are peers in a sense, because they are both public parts of the business layer.
Were project.Resources to create a copy of the collection, it would (by definition) have also created a copy of all the child (and possibly grandchild) objects - in other words the whole graph. No sane person would write code like that, because the cost of cloning entire object graphs is quite high.
But on top of that, the Assign method would then be called on this cloned object graph. The resulting changes would be made to a copy, not to the real objects. It isn't clear how those changes could be persisted at that point? You'd have two (or actually n) copies of this graph that would have to be reconciled when the user clicks the Save button. The complexity of such a concept is quite high.
Now there is another entire world view on object design. In that world view UI code doesn't interact with CSLA style business objects at all - ever. In that world view there are a set of very high-level objects that represent use cases. You can almost think of these "objects" as a service interface, because they tend to return and consume entity objects (simple data containers). All the actions taken on those entity objects are entirely encapsulated behind this high-level interface.
Personally I find that design approach to have limited value, at least when creating Windows or WPF interfaces. They totally eliminate any hope of interactivity thanks to that thick layer of abstraction.
If you are looking for that style of OO design, then CSLA .NET simply isn't for you. CSLA .NET goes the other way: helping you build a set of objects that reflect the problem domain, in such a way that you should interact directly with those objects. The result is a high level of interactivity, and yet the actual business logic and processing remains (to a very large degree) encapsulated within the set of business objects the comprise the business layer.
For years, OO programming "standards" have dictated that references (or pointers) to private mutable data members should not be returned by methods (in this case the Project.Resources property getter is doing just that - returning a reference to a private instance of the ResourceList class). I agree it is very cumbersome to avoid this. All I can think of is that you have to (1) implement a facade on the parent class that replicates the behavior of all the children (at least for the methods that return references to mutable types); or (2) make a copy (using your cloner) on the getter, allowing the caller to modify a copy of the internal data, and then provide a setter (sometimes called a mutator) that reassigns the private reference to the one the caller modified ... but the caller still has a reference to it (yuk). This is why I am looking for a better way :-)
Maybe this is just a trade off we make for convenience and performance.
Dozens of references are avaiable that state that this is a violation of encapsulation:
Just do a Google Search on "do not return" private data.
Also in dozens of Java books and "Effective C#" by Bill Wagner.
Thanks.
CSLA .NET objects are designed to be interacted with. The objects are not private data. All public objects are just that: public. The properties are merely a navigation path to get to those objects.
It is important to distinguish between data that's actually private, and data that exists to provide a navigable path.
UML has a notation for this. The aggregation symbol has an associated (and optional) navigation symbol. A great many aggregate objects also provide navigation to their "children". And that's exactly what's happening here.
A Project contains (aggregates) the ProjectResources collection, which in turn contains ProjectResource objects. But none of these objects are private data. They are all public, and are designed to be public. The properties merely provide a navigation structure through the object graph to make it possible for a consumer to reach each object.
It is equally possible, and reasonably common, for an object to collaborate with other objects that are not navigable. Sometimes those collaborators are public, sometimes private. But if there's no natural navigable relationship then the navigation link shouldn't exist. And of course if the collaborator really is private data then it shouldn't be exposed at all.
But again, that's not the case here. The Project object consists of a set of public objects that exist within the context of a navigable object graph.
In your sample solution, there is a Project class. That class has a private data member called "resources" that "points to" a mutable reference type (not an immutable type or a value type).
<code>
private ProjectResources _resources =
ProjectResources.NewProjectResources();
<end code>
There is a property ("Resources") who's getter clearly returns a reference to that private member variable.
<code>
public ProjectResources Resources
{
get { return _resources; }
}
<end code>
This breaks encapsulation.
Given this, I do not understand your comments above above being public - it is clearly private. The Property is public, yes, but it is returning a reference to a private member variable.
The way it is coded, you might as well make "_resources" public - which is maybe what you wanted to do so it could be a "navigable object graph". But the way it is currently implemented gives rise to the question of whether you mean it to be private (as the declaration is coded) or public (as the behavior is coded .. by giving out a reference to the private memeber variable).
Thanks for the responses.
Your comment from above is inserted below:
<start excerpt>
The instance of the ResourceList object is held as a private field for the following reasons (maybe more)
<end excerpt>
In response to your first bullet point: Making a member variable private actually prevents navigation to it - it is the public getter of the property that makes it visible outside the class. That is not a "reason to make it private". That doesn't make sense.
In response to the second bullet: making a member variable private does not make it readonly. It is not marked readonly (or const) anyway.
I am at a loss as to why no one (yet) seems to acknowledge that when you return a reference to a mutable reference-type object in the .NET framework (as is happening in this code), you are giving the caller a reference to the very same object that you are saying is private in the _resource declaration. This allows the holder of that reference to invoke all the public methods on the "private (encapsulated)" instance variable. You have essentially made a private instance variable public.
Race conditions (that you mention above) is a threading issue. Race conditions are avoided with proper thread synchronization techniques. It has nothing to do with the class design and definitely not related to encapsulation or copying.
J Phillips:
Just do a Google Search on "do not return" private data.
I tried that, and some variations. Everything I found talks about how you should not expose a reference to a mutable private object when it is not your intention to allow properties of that object to be changed from outside the parent object. Most of the examples I found were in Java. It seems that Java has a Date object that has a SetTime() method which changes the time inside the Date object. A common error is to expose a Date object as a public readonly property while forgetting that someone could call SetTime() on the object therefore changing its internal data. That's a classic example of violating the rule you are talking about. (btw, the .NET DateTime struct does not have that problem. All its Add() methods return a new DateTime.)
A common .NET example is exposing an array as a readonly property and forgetting that anyone can change an array element.
The situation with Rocky's CSLA examples is different. We actually want the outside world to be able to modify the collection. That's kind of the whole purpose of exposing it as public. If you don't want the outside world to be able to modify the collection then derive the collection from CSLA's ReadOnlyListBase.
In response to the second bullet: making a member variable private does not make it readonly. It is not marked readonly (or const) anyway.
You seem to be confusing the reference to the collection with the contents of the collection. Brian is exactly right. You don't want someone to be able to replace the collection with a new collection. That would definitely break encapsulation and it is the reason why the public property only has a getter. You would not have that protection if the private variable was exposed.
From your first post
Any suggestions on how to do this better - avoid violating encapsulation and yet provide the desired behavior?
Please give a real world example of what you are trying to avoid? I'm no OO expert but, to me, in simple terms, encapsulation means locking up the internals of my objects so nobody can fiddle with the way I choose to implement a method or property. That means I can safely change it later without breaking other code.
Cheers
Ross
I was under the impression that encapsulation was to prevent direct access to class member variables. But, in order to interact with the class, one would need to use a specific set of methods, i.e. property getters and setters. I don't see how the above use can possibly break encapsulation.
UI developers can interact with the business objects, but only through very specific channels. Hence, encapsulation is upheld.
Also, the exact details of how the resource list is generated is concealed. The UI developer only knows to get the list, add to the list, or delete from the list. Again, encapsulation is upheld.
The way it is coded, you might as well make "_resources" public - which is maybe what you wanted to do so it could be a "navigable object graph". But the way it is currently implemented gives rise to the question of whether you mean it to be private (as the declaration is coded) or public (as the behavior is coded .. by giving out a reference to the private memeber variable).
I think you misunderstand how a .NET property works.
Exposing _resources directly would allow a caller to change the field to point to some other object - and that would be bad.
Exposing a reference to the ProjectResources object through a property set is merely an implementation of an accessor method. The caller can not change where _resources points, all they can do is interact with the object provided. That object, of type ProjectResources, is designed to be used by consumers of the business layer. It has a safe public interface that is designed for consumption.
Copyright (c) Marimer LLC