does model break encapsulation?

peterc's Avatar

peterc

18 Feb, 2011 07:28 AM

Hi, I was reading a very informative discussion on why not to injecting a model into a mediator: http://knowledge.robotlegs.org/discussions/questions/352-stray-clar... . I notice that Stray mention that a model is state + logic. It is very similar to OOP concept of encapsulation: http://en.wikipedia.org/wiki/Encapsulation_%28object-oriented_progr... . This is actually the one point I am not sure about. In the past I always assumed one of the main reason for not injecting a model in a mediator is because the model break encapsulation. And accessing indirectly through commands besides providing the loosely coupling benefit will also help somewhat patch up the encapsulation problem from the mediators point of view.

The reason I think model break encapsulation is because model often have a public setter method to set VO. That basically provides it the ability to change the internal state of a model object without any protection. For example, if you have shopping cart model with the operations addItem, deleteItem, and updateItem. The shopping cart model can be kept in a consistence state if only those methods are called. However you can also use the setter method to set the internal state to anything you want. With that, the model seems to me as safe as a global object with public properties.

The reason to have such setter method seems to be unavoidable. Services need to load the data in the model once it is finish retrieving them. More often, the operations on the services are similar to the operations of the model though. For example: a shopping cart service will have something like addItemToCart, deleteItemFromCart. And services usually have a addition method to retrieve all the items of cart remotely and set the value in the cart model. So I was thinking if I have the model call the methods in the services instead of the having service setting data on the model. That way I can get rid of the public setter method in the model and replace with a getCartContent method and solve the encapsulation problem. In that way, a command calls the model.getCartContent method, the model.getCartContent method calls the service.getCartContent method and return a AsyncToken(or a Promise). When the service received the data, it processes it and convert it to a value object and put in it in the AsyncToken(or a Promise). The model takes the value and updates its internal state. The service kind of becomes a helper class of the model. I think this way is less procedural and more object oriented and it is easier to understand because the command will be dealing with a object and the operations that act on it, instead of data and some utility function that happen to manipulate data that belong to different object(the Shoppingcart model). One of the unfortunate consequences of this is now models have a dependency on services. A happy consequence of this is services now have no dependency on model and can be reuse by different commands and models.

It will be nice to have people’s opinions on this. Is this too complicated a fix for the encapsulation problem? Is there actually a encapsulation problem? Does this way of doing things cause more problems then it solved? Does this make the model too heavy and is this a bad design?

  1. 1 Posted by krasimir on 18 Feb, 2011 08:21 AM

    krasimir's Avatar

    Hello peterc,

    first of all I'm also thinking that injecting the model in the mediator is not the very best option. If your mediator needs data and this data is saved in the model there is no need to send the whole model. You can always dispatch a custom event with payload that holds the necessary stuff. I don't think that calling a method of the service from the model is a good idea. It's because in this case you should use this model with only this service or at least you should have a method with absolutely the same name in all your services. The custom events are really helpful here. Let's take your example (with the shopping card). When the data is received by the service better simply dispatch an event which could be caught by the model. The model parses the data and fills itself with VOs. If you want to perform some action on a specific VO from the model, i.e. to delete or add a new item to the card use the shared event dispatcher. For example: dispatch(new ShoppingCardEvents(ShoppingCardEvents.ADD_ITEM, newItemVO));
    In this case the class (command, service, mediator or something else) doesn't care who will catch this event. Only the model listen for that and update its state. And at the end you have
    - service that is not tightly connected to the model - model that only listen for events, i.e. doesn't depend from the service

  2. Support Staff 2 Posted by Stray on 18 Feb, 2011 10:05 AM

    Stray's Avatar

    It's perfectly possible for the model to run checks on the data it is passed through its setters - whether as a vo or as individual values. To pass a vo to the model my method is usually something like

    updateModelFromVO(vo:SomeModelVO)
    

    And because I use Commands, if the values need to be checked (because they come from a user input form for example) then usually there is a form validator that is used in the Command before the data is submitted to the model. But the model can do its own checks on values as well and usually does.

    So... I don't see that there is problem with model encapsulation generally.

    In fact, I couldn't find anywhere in the thread that talked about encapsulation at all... I know we talked about 'state plus logic' but I'm not sure that's the same thing as encapsulation - I was more trying to define the edges of what is / isn't a model.

    To be honest my objection to injecting models into mediators is based on a combination things:

    1) Because mediators are created and destroyed every time the view they mediate is added / removed, it is sensible to make our mediators as lightweight as possible.

    2) If you inject a model into a mediator then it is very tempting to begin to use the model in more and more complex ways in order to 'quickly' do things.

    3) If you wind up with logic in your mediator layer then your mediators are no longer lightweight.

    4) The actual logic that makes your application work is now dispersed in various mediator handler functions around your application. You probably have repetition.

    5) Usually handlers are not well named - we have "submitClickedHandler" and so on, and this means that somebody reading the code (a colleague or even yourself a few weeks later) actually has to read the detail of what the code does to know what is being done. (Compare this with proper method naming such as 'cleanExtraWhitespaceFromUserName' ).

    6) Once you have logic in your mediators it's also very tempting to add state to your mediators. If you do add state, and then you pass this state to the model, it's possible to end up interfering with garbage collection if you forget to clean up after yourself in onRemove() . If you do everything via the eventMap then there are no garbage collection problems.

    7) If you also need to listen for update events on the model, but you inject it as well, you end up doing the same 'work' (updating view to match model state) in 2 different ways.

    8) Making changes is more difficult because your application logic and your view behaviour are now mixed in one class, and if you decide that the view should arrive later, or your model should arrive later, you run into timing problems.

    9) Your mediator now has 2 responsibilities, and if either your model API or your view API change you have to change your mediator.

    To be honest it's mostly based on experience. Shaun and Joel would both see injecting the model into the mediator as acceptable, provided you then restrain yourself in how you work with it. In my experience people aren't so good at that kind of self-restraint :) When I first started using robotlegs I thought that it was ok to inject a model into a mediator - but I eventually changed my mind.

    I've got a very large robotlegs project myself, and I've helped out others with large robotlegs projects where they've got a problem and the source of hard-to-fix problems is usually application-logic-in-the-mediator.

    On a small project you can probably do anything you like and get out before the technical debt catches up with you - but in a multi-month or even multi-year project you have a responsibility to the client to be able to keep making changes in an efficient way.

    So - on to the suggestion you made...

    As you say - and Krasimir further illustrates - the problem with having the model call the service is that then the model is coupled to the service - and maybe the model is also doing two jobs (because it is now concerned with running updates as well).

    I like to use this approach: http://www.twitpic.com/40tq9x

    (The view in this diagram represents the whole view layer - so mediators + actual views).

    I almost always use a factory to process data coming in from a service, and the factory will then update the model - but not directly.

    The service only knows about the factory interface API, the factory only knows how to build or update the model, and the model knows nothing about anybody else.

    Which is quite nicely decoupled IMO. The service has no dependency on the model - only on 'something' that is going to process its result.

    I could even further decouple this I guess by using a promise between the service and the factory... that's an interesting thought.

    I'd disagree with what Krasimir says about the model listening for events - in my world models and services only dispatch events, they don't listen. And only mediators and commands respond to events, though they dispatch them too.

    I'm talking about application events obviously - the service may well listen to the events of a Loader, but it doesn't listen for application events.

    But really the idea is the same - you minimise coupling by having events glue everything together instead of API. The only classes that use model and service API are Commands and Factories/helpers, and the view API is used only by its mediator.

    I like to keep things simple :)

    Stray

  3. 3 Posted by krasimir on 18 Feb, 2011 10:24 AM

    krasimir's Avatar

    Yep, Stray is right. It is better to use command to populate the model with the received data, instead of listening for event coming from the service. It will be better for tracing the processes, because we know where exactly comes the data.

    @Stray what you think, where is better to place the parsing of the data. I mean if we have a xml that is loaded by service and has to be parsed: 1. Send the xml to the model, which parse it and convert it to VOs
    2. Parse the xml in the command (that is fired when the data arrives), convert it to VOs and send these VOs to the model

  4. Support Staff 4 Posted by Stray on 18 Feb, 2011 11:09 AM

    Stray's Avatar

    Hi Krasimir,

    I do neither - my services have a factory injected against an interface.

    The factory usually has a single public method like processXML - or if I am using the same factory for several services because there is a lot in common it might have processUserXML and processContentXML.

    Usually the factory makes use of some helpers specific to each data type like
    UserXMLToModelProcessor and so on. The builder/helpers only have two functions: buildFromXML (or buildFromJSON or whatever), and a getter for an errorMessage. If the data comes back null then the factory gets the errorMessage and dispatches a custom error event. The application (through Commands) then decides whether it needs to worry about this error or not - sometimes it's important, sometimes it's not critical.

    So the factory takes the XML it was passed by the service (or sometimes URLVars) and it splits it into the pieces to be processed and uses the various data-type-specific builders to turn it into the data objects to use to populate the model.

    So - the service only knows how to load the data. The factory only knows how to process the data. If the factory needs to create more than one kind of object then each creation is done with a helper class that only knows how to build that particular object.

    This probably best illustrates the relationship between the Factory and the actual builders:

        public function processCompanyData(dataXML:XML):void 
        {
            var dataModelBuilder:CompanyDataModelXMLBuilder = new CompanyDataModelXMLBuilder();
            var dataModelVector:Vector.<CompanyDataModel> = new Vector.<CompanyDataModel>();
    
            for each(var itemXML:XML in dataXML.d)
            {
                var dataModel:CompanyDataModel = dataModelBuilder.buildFromXML(itemXML);
                if(dataModel == null)
                {
                        dispatchBuildError(dataModelBuilder.errorMessage, CompanyDataModel);
                }
                else
                {
                        dataModelVector.push(dataModel);
                }
            }
    
            dataModelVector.sort(sortOnTitle);
    
            var dataSetModel:CompanyDataSetModel = new CompanyDataSetModel(dataModelVector);
    
            completeInjections(dataSetModel, CompanyDataSetModel);
        }
    
        protected function completeInjections(instance:*, instanceClass:Class):void
        {
            injector.injectInto(instance);
            injector.mapValue(instanceClass, instance);
        }
    

    You can see here that I'm using the injector to wire things up, and that I've abstracted this part to another function - that's the beauty of injecting against the factory interface in the service. If I wanted to move this outside of robotlegs I would only have to override the function that does the injectInto / mapValue part. All the logic of creation of the company data etc is sealed off from that change.

    The service sends updates during loading. The factory will send error events if the data is broken. The model sends update events after it has been updated. The helper classes that build each data type aren't framework-connected - they are short lived objects that just get used to turn a piece of XML into an object and then are thrown away.

    The advantage of this approach is that you can use the same process each time. I have an app that allows people to edit and administrate many different kinds of data on their system. The basis for the loading, saving and deleting services for each different type of data is entirely abstract - and then there are only tiny variations in terms of which actual factory, and what URLVars are sent etc.

    All of that variation is done through a look-up based on the type of data that is requested / saved / deleted. Obviously that type is a Class and not a magic string :)

    It's just abstract factory / template method patterns in combination I think. I like it a lot because once it's working it just keeps working - the process is tested, and you only have to add and test each of the little variations.

    The only part that I'm not happy with is the use of the check against 'null' to decide whether the builder hit an error. I'm sure there is a better way to do this, but I haven't worked out what that is yet! Any input there gratefully received...

    I guess I could pass a promise to the builder and use handlers on the onResult , onFault etc, but as this part of the process is currently synchronous I'm not 100% sure about that in terms of how it could work in a loop.

    Sorry... that's a very long answer to a short question!

    Stray

  5. 5 Posted by krasimir on 18 Feb, 2011 11:27 AM

    krasimir's Avatar

    :) great answer and good explanation. I don't think that there are many options to check if the builder parse the data successfully or not. As you said the process is synchronous.

  6. 6 Posted by Weyert on 18 Feb, 2011 01:27 PM

    Weyert's Avatar

    Sounds like a good approach to let the factory based on the VO to submit which query strings needs to be send along. This way you could keep service the same and just swap out the factory when one small change is needed by a single project.

    For example, instead of only sending firstname/lastname you should also send a unique identifier and an email address. This should save a lot of overrides.. This URLVariables factory only needs to be aware of the model the VO data of course.

    In a few of our game projects at work we just slightly need to change which data gets send to the game server based on the score submission form. E.g. address or not, uid for use with social networks etc.

  7. 7 Posted by peterc on 18 Feb, 2011 04:16 PM

    peterc's Avatar

    thanks everyone for the great replies. I got my answers and more. The factory especially is a interesting topic I certainly look into further for my project.

  8. Stray closed this discussion on 18 Feb, 2011 06:56 PM.

Comments are currently closed for this discussion. You can start a new one.

Keyboard shortcuts

Generic

? Show this help
ESC Blurs the current field

Comment Form

r Focus the comment reply box
^ + ↩ Submit the comment

You can use Command ⌘ instead of Control ^ on Mac