6

Normally when I have some data to store, I do something like

Model model = new Model();
Notifier notifier = new EventBus();
Store store = new DataStore();
model.updateTitle("new title");
store.save(model);
notifier.notifyAboutChange(model);

I have read a lot of posts about putting the logic into the models, but people seems to still save their updated model "outside" the model which results in code as above, where you notify about the change outside the model.

Why not save directly in the model? Then the above code could be something like

Model model = new Model(new EventBus(), new DataStore());
model.updateTitle("new title");
// sample model
class Model {
    Constructor(Notifier notifier, Store store) {..}
    void updateTitle(String str) {
        this.title = str
        this.store.update(this);
        this.notifier.notifyAboutChange(this);
    }
}

This way you are assured that everything is done in everytime updateTitle is invoked and make the calls to the models simpler.

One problem with this might be that objects are often automatically constructed by orms, which means that you don't have access to the constructor/they are constructed automatically. This could be solved by injected into the updateTitle method.

  • 1
    It sounds like the question is more about "why separate persistence from business logic" then? – Greg Burghardt May 31 '17 at 16:32
  • Yeah, or at least: why do it two different places? – Anders Jun 2 '17 at 9:31
9

When you use

void updateTitle(String str) {
    this.title = str
    this.store.update(this);
    this.notifier.notifyAboutChange(this);
}

you are grouping two operations into one -- updating the model in memory and making the updated model persistent. This is a policy decision you have to make for your application. Only you and your team are in a position to decide whether it makes sense for your application.

If you decide that updating a model in memory and making the updated model persistent needs to be one user function, there are ramifications. If you have N functions that update various aspects of the model, you will need to add the same code in all the other functions too.

As a principle, it makes sense to separate those operations. This allows the user to make as many updates to the model as they like before choosing to make it persistent.

However, as Google Docs and other online applilcations have shown, making the model persistent as soon as it is updated by the user is not such a bad thing. It might make sense for your application too.

Even if you choose to adopt the policy of saving the model as soon as it is updated, I would recommend using a event based mechanism and let an observer make the model persistent as soon as they receive the change notification event.

void updateTitle(String str) {
    this.title = str
    // Let an observer listen to this event and save the model.
    this.notifier.notifyAboutChange(this);
}
  • Thanks for your reply, @Rsahu. I think my problem is, that I have to notify about different events when saving. It's is important to notify specifically when the title has changed, i.e. by TitleChangedEvent and so on. If i just change the title on the model and persist it using my repository, it wouldn't know (simple) what has changed, and therefor it is hard to notify about this. – Anders Jun 14 '17 at 17:00
  • @Anders, the granularity of the change events can be as fine (or as coarse) as you need them to be. A generic modified event, by design, will be harder to encapsulate fine grained changes. – R Sahu Jun 14 '17 at 17:57
  • Yeah exactly. My concern is more how to actually capture which changes has been made, so the right event(s) can be fired when persisted. – Anders Jun 14 '17 at 20:07
  • @Anders, I see your problem. What I am suggesting is different. You don't need to send out any events after persisting the model. Persisting the model is another side effect that responds to an event. – R Sahu Jun 14 '17 at 20:12
  • Thank you, I see your point :) Maybe actually two events - one event before it is stored/persisted (UpdateTitleRequest) and an event afterwards - TitleUpdated. This would prevent "false" events from being sent, should the database break down. – Anders Jun 14 '17 at 20:19
2

It is very common that the model is responsible for persisting itself: that's called the Active Record pattern.

Active Records are super convenient to use, and can often be autogenerated by an ORM. This approach seems especially common in web frameworks like Ruby on Rails.

There is a slight difference to your code: an Active Record has a separate .Save() method, and does not save automatically. Why? While we are working with the model, we might be making various changes to it. After we are done, we want to save all our changes to the database in a single transaction. Should an exception occur, we do not want to save the changes leading up to the exception.

If we would save after each change, this would not only lead to much more database queries, it would possibly also write incomplete or invalid data to the DB. This difference does not seem important when you only change a single property, but becomes really critical if you update multiple related fields, possibly even across multiple objects.

While Active Records are popular, they do have one big problem: the model needs a database connection. This generally means that you can't write unit tests for your model behaviour without mocking the database, which may be difficult to do. Separating persistence from the model e.g. by using the Repository pattern is generally considered to be much cleaner. That way, the model only represents some entity of your domain model with its intrinsic business logic, and the repository only manages model persistence.

  • Thanks for your input. My concern with the repository is that I often feel like repeating myself, because a lot of the logic already exists in the model. Another concern is that i want to publish events (on my eventbus) when something has changed, which of cause could be done by the repository. My problem is that I have different methods on my model, which should publish different events (for instance "statusUpdated", "passwordChanged") and things like that. This would put too much logic into the repository. – Anders Jun 2 '17 at 9:25
2

I have read a lot of posts about putting the logic into the models, but people seems to still save their updated model "outside" the model which results in code as above, where you notify about the change outside the model.

Yep, that is how MVC works. The model is supposed to be a dumb container with no behavior. All that injection you're doing, and adding methods for persistence, that is outside the pattern.

Why keep models behaviorless? Two big reasons come to mind.

  1. Model binding, data annotations, and scaffolding can be used to automatically populate models from the HTTP request (e.g. form/post variables). The model binders won't know about your updateTitle method, they will just set the Title. Heaven help you if you then use that same model as a domain object in a repository update-- it'll be populated with values from the request, not the data store, and you may end up with unforeseen data integrity problems.

  2. Unit testing is much easier with behaviorless models because they form a layer of isolation between controller and view. What's more, it's a layer that does not need to be shimmed, stubbed, or mocked (because it has no behavior). That is why models don't normally have interfaces. Now you are going to need an interface for each and every model, so that you can stub them in your unit tests.

If your scheme somehow offsets the benefits of the normal MVC pattern, by all means, go ahead. Personally I would urge you to consider designing your site with behaviorless models. Keep the persistence logic in the controller (and its dependencies) where it belongs.

  • 3
    When you speak of "models" specifically it seems you are talking about "View Models". Business logic belongs elsewhere. It sure doesn't belong in a controller or view model. – Greg Burghardt May 31 '17 at 19:39
  • 1
    Nope, not talking about View Models, and nowhere in my post is the phrase "business logic." I am not sure what point you are trying to make. – John Wu May 31 '17 at 20:05
  • 1
    "Model" is a fuzzy concept now. Many times busines logic goes in some sort of model (View Model, Domain Model). I think your answer needs to address this (where business logic goes). – Greg Burghardt May 31 '17 at 20:29
  • It is not fuzzy in this context one bit. The post is tagged "MVC" and Model is the M. It is clearly defined both by pattern and by practice. But now I understand your comment better, thank you. – John Wu May 31 '17 at 20:30
1

If notifications are important for the business layer of your application, model should do them itself:

// Initialization
Notifier bus = new Notifier();
Repository repository = new Repository(notifier);

// Domain code
Transaction transaction = repository.openTransaction();
Model model = repository.getNewModel();
model.setTitle("new title");
transaction.commit();

Note how the model itself gets to decide whether to send out notifications, how and when to persist a change. Also notice, that model does not know about actual implementation of persistence layer or event bus. They are supplied to it using dependency injection as external dependencies.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.