views:

151

answers:

7

I am only allowed to use private members in the programming course I'm taking, and I was wondering if something like this is ok.

public class View {
    private Model object_;
    public View(Model object) {
       object_ = object;
       //blah blah blah
    }
//blah blah blah
}

public class Controller {
    private Model object_;
    public Controller(Model object) {
       object_ = object;
       //blah blah blah
    }
//blah blah blah
}

public class MainClass {
    public static void main(String [ ] args) {
        Model m = new Model();
        Controller c = new Controller(m);
        View v = new View(m);
        //blah blah blah
    }
}

The View and Controller classes both hold the same instance of the Model as private fields. Is this acceptable? It seems like it would violate the concept of private fields. I tried asking my professor, and he said it was ok, but I'm not sure if he understood my question, or if I understood his answer :)

A: 

As far as I know, there's no hard and fast "law of private fields" that says that an object can be held privately by more than one class. For something like an immutable reference class, it makes sense that multiple class instances could receive the immutable reference class and hold it as a private attribute.

In your example, Carl is right -- Views and Controllers should generally take a Model as a parameter in some method, not keep a reference to them. So the code would look more like this:

public Response handleRequest(Request request) {
    Controller c = new Controller();
    Model m = c.processRequest(request);
    View v = new View();
    Response response = v.createResponse(model);
    return response;
}
Kaleb Brasee
+1  A: 

If you look at this article, you'll see that ideally the controller sits between the model and view. So I would perhaps reorganise your dependencies somewhat (there's some disagreement about this - see below)

But the answer to your question (regardless of the domain) is that you can share these private references around. Ideally they should be references to interfaces, and the implementations can change without changing the dependent classes. So (for example) you would have a Model interface, and an implementation ModelImpl, and the shared references would be of type Model. You can then substitute different implementations at a later date without affecting the majority of your code.

Brian Agnew
Controllers do not sit between the model and view (including in the linked article). Views render models. Controllers update models. The trigger that tells a view to re-render is sometimes different (either controller or model) but is typically the model via the Observer pattern.
SingleShot
Oh, and I agree with the rest of what you say ;-)
SingleShot
I noted that the article references direct and indirect associations, and I've seen different implementations, hence my cautious 'perhaps' above. I think the core of the answer is the 2nd paragraph (which I'm pleased is less contentious :-)
Brian Agnew
A: 

Yes, this is acceptable, private fields should be used to enforce encapsulation, by controllingb access to these field through getter/ setter methods if required. if these fields where public or even package then outside classes may start to depend upon you implementation of the class, which ties you down in terms of refactoring methods etc. Allowing other classes access to fields they shouldny have access to can provide a privacy leak.

If model where public.

Class messUp {

Model aModel = object_; aModel.doWhatTheHellIWantToYourModel();

}

this could completely ivalidate the state of your system.

chrisg
sorry editModel aModel = instance.object_;all i would need to acces your model is a reference to a class which declares it public.
chrisg
+2  A: 

Regarding the title question: yes, it's ok. Think of a tree where siblings share the reference to a parent. This situation is quite common.

clorz
+1  A: 

The short answer is that it is fine.

The longer answer (forgetting about MVC for a second, because it seems that MVC is an example of doing it, not the core of the question) is you have to think about mutability. If an object is not mutable (none of the fields on the object can ever change) then sharing is a very good thing, it reduces memory footprint and has no real downside.

If, however the object is mutable, then you have to understand how changing the state of the object affects those that reference it. In MVC it is pretty typical to see the model changed and then have the view ultimately adjusted based on that, so you would build that into the design, but consider this case:

  private Collection myCollection; 

  public void setMyCollection(Collection someCollection) { 
       myCollection = someCollection; 
  }

Such code has to be very confident that the caller won't pass it a mutable collection that gets changed afterward, or it has to allow for that. The typical way around the problem in the above case is to make a copy (myCollection = new ArrayList(someCollection);)

So to sum up, it is a common practice that is fine, but when the object is mutable, you have to think through the consequences on your design.

Yishai
+1  A: 

Short answer: There isn't anything wrong with two different classes which have private member variables that reference the same thing. It is quite common.

Long Answer: My guess is that what your professor may have been talking about here is:
private Model object_ is an object reference variable (not a primitive). So when you pass object_ into the constructors of Controller and View, you are passing a "link" to the same instance of
object_, instead of passing a copy of object_.

Whether or not you choose to store them as private in Controller and View is a completely different decision. I am familiar with university programming courses forbidding the use of non-private member variables (been there). They want to introduce some Object Oriented concepts, such as encapsulation (later), they will then tell you to create public accessors and mutators for the private member variables. This is generally be a good principle to start with, as protected and public member variables should only be used when you have a good reason to do so.

So yes, these is correct Java, but whether or not this actually what you want to do with your code is another matter (see Caleb's answer).

bguiz
+1  A: 

Is this acceptable?

Yes it is.

It seems like it would violate the concept of private fields

It doesn't. The controller cannot change the instance from the view, because it it private.

Consider the following line of code that attempt to change the model in the view:

public class Controller {
    private Model object_;
    public Controller(Model object) {
        object_ = object;
        this.object_.view.object_ = new Model(); // doesn't work.
        // or view.object_ = ....            
       //blah blah blah
    }
    //blah blah blah
}

Well this won't work, 1st because, the model doesn't have a reference to the view, and 2nd, if it had it, the controller can't change the model in the view, because it is invisible to him.

Sharing the same object may not be desirable in some situations. For instance if you rely on the state of a given object and that state is change by other object, your code may fail. See Defensive Copying That's why it is recommended to use immutable objects and restrict the access to it.

But particularly in the MVC design pattern sharing an object that's exactly what you need, or at least sharing the state of that object. In MVC what you need is the View and the Controller to be aware of the Model state. Seeing the same object doesn't make them less private ( in the sense that neither of each objects know they are seeing the same thing ) this allows Low Coupling

What would definitely ruin object privacy would have that attribute as non private as:

class View {
   public Model m; 
}
class Controller {
    public Model m;  
}
....
public static void main( String ... args ) { 
     Model m = ... 
     View v = new View();
     Controller c = new Controller();
     v.m = m;
     c.m = m;
}

Because there won't be any access control that prevents from wild changes to occur.

OscarRyz
Ok, thanks y'all. I wasn't exactly sure how far privacy was supposed to extend. I.E. I wasn't sure if member objects were supposed to change states without the class holding the object initiating the change... If that makes any sense.
ektrules