Architecture: Modifying the model in different ways

Question

Problem statement

I have a model class that looks something like (extremely simplified; some members and many, many methods omitted for clarity):

class MyModelItem
{
public:
    enum ItemState
    {
        State1,
        State2
    };

    QString text() const;

    ItemState state() const;

private:
    QString _text;

    ItemState _state;
}

It is a core element of the application and is used in many different parts of the code:

• It is serialized/deserialized into/from various file formats
• It can be written into or read from a database
• It can be updated by an 'import', that reads a file and applies changes to the currently loaded in-memory model
• It can be updated by the user through various GUI functions

The problem is, this class is has grown over the years and now has several thousands lines of code; it has become a prime example of how to violate the Single responsibility principle.

It has methods for setting the 'text', 'state', etc. directly (after deserialization) and the same set of methods for setting them from within the UI, which has side effects like updating the 'lastChangedDate' and 'lastChangedUser' etc. Some methods or groups of methods exist even more than twice, with everyone of them doing basically the same thing but slightly different.

When developing new parts of the application, you are very likely using the wrong of the five different ways of manipulating MyModelItem, which makes it extremely time consuming and frustrating.

Requirements

Given this historically grown and overly complex class, the goal is to separate all different concerns of it into different classes, leaving only the core data members in it.

Ideally, I would prefer a solution where a MyModelItem object has nothing but const members for accessing the data and modifications can only be made using special classes.

Every one of these special classes could then contain an actual concrete implementation of the business logic (a setter of 'text' could do something like "if the text to be set begins with a certain substring and the state equals 'State1', set it to 'State2'").

First part of the solution

For loading and storing the whole model, which consists of many MyModelItem objects and some more, the Visitor pattern looks like a promising solution. I could implement several visitor classes for different file formats or database schemas and have a save and load method in MyModelItem, which accept such a visitor object each.

Open question

When the user enters a specific text, I want to validate that input. The same validation must be made if the input comes from another part of the application, which means I can not move the validation into the UI (in any case, UI-only-validation is often a bad idea). But if the validation happens in the MyModelItem itself, I have two problems again:

• The separation of concerns, which was the goal to begin with is negated. All the business logic code is still 'dumped' into the poor model.
• When called by other parts of the application, this validation has to look differently. Implementing different validating-setter-methods is how it is done right now, which has a bad code smell.

It is clear now that the validation has to be moved outside both the UI and the model, into some sort of controller (in a MVC sense) class or collection of classes. These should then decorate/visit/etc the actual dumb model class with its data.

Which software design pattern fits best to the described case, to allow for different ways of modifying the instances of my class?

I am asking, because none of the patterns I know solves my problem entirely and I feel like I'm missing something here...

Thanks a lot for your ideas!

Answer 1

Plain strategy pattern seems the best strategy to me.

What I understand from your statement is that:

1. The model is mutable.
2. the mutation may happen through different source. (ie. different classes)
3. the model must validate each mutation effort.
4. Depending on the source of an effort the validation process differs.
5. the model is oblivious of the source and the process. its prime concern is the state of object it is modeling.

Proposal:

1. let the Source be the classes which somehow mutate the model. it may be the deserializers, the UI, the importers etc.
2. let a validator be an interface/super-class which holds a basic logic of validation. it can have methods like : validateText(String), validateState(ItemState)...
3. Every Source has-a validator. That validator may be an instance of the base-validator or may inherit and override some of its methods.
4. Every validator has-a reference to the model.
5. A source first sets its own validator then takes the mutation attempt.

now,

Source1                   Model                  Validator
   |     setText("aaa")     |                        |
   |----------------------->|    validateText("aaa") |
   |                        |----------------------->|
   |                        |                        |
   |                        |       setState(2)      |
   |          true          |<-----------------------|
   |<-----------------------|                        |

the behavior of different validators might be different.

Answer 2

Although you don't state it explicitly, refactoring thousands of lines of code is a daunting task, and I imagine that some incremental process is preferred over an all-or-nothing one.

Furthermore, the compiler should help as much as possible to detect errors. If it is a lot of work and frustration now to figure out which methods should be called, it will be even worse if the API has been made uniform.

Therefore, I would propose to use the Facade pattern, mostly for this reason:

wrap a poorly designed collection of APIs with a single well-designed API (as per task needs)

Because that is basically what you have: a collection of APIs in one class, that needs to be separated into different groups. Each group would get its own Facade, with its own calls. So the current MyModelItem, with all its carefully crafted different method invocations over the years:

...
void setText(String s);
void setTextGUI(String s); // different name
void setText(int handler, String s); // overloading
void setTextAsUnmentionedSideEffect(int state);
...

becomes:

class FacadeInternal {
    setText(String s);
}
class FacadeGUI {
    setTextGUI(String s);
}
class FacadeImport {
    setText(int handler, String s);
}
class FacadeSideEffects {
    setTextAsUnmentionedSideEffect(int state);
}

If we remove the current members in MyModelItem to MyModelItemData, then we get:

class MyModelItem {
    MyModelItemData data;

    FacadeGUI& getFacade(GUI client) { return FacadeGUI::getInstance(data); }
    FacadeImport& getFacade(Importer client) { return FacadeImport::getInstance(data); }
}

GUI::setText(MyModelItem& item, String s) {
    //item.setTextGUI(s);
    item.getFacade(this).setTextGUI(s);
}

Of course, implementation variants exist here. It could equally well be:

GUI::setText(MyModelItem& item, String s) {
    myFacade.setTextGUI(item, s);
}

That is more dependent on restrictions on memory, object creation, concurrency, etc. The point is that up till now, it is all straight forward (I won't say search-and-replace), and the compiler helps every step of the way to catch errors.

The nice thing about the Facade is that it can form an interface to multiple libraries/classes. After splitting things up, the business rules are all in several Facades, but you can refactor them further:

class FacadeGUI {
    MyModelItemData data;
    GUIValidator validator;
    GUIDependentData guiData;

    setTextGUI(String s) {
        if (validator.validate(data, s)) {
            guiData.update(withSomething)
            data.setText(s);
        }
    }
}

and the GUI code won't have to be changed one bit.

After all that you might choose to normalize the Facades, so that they all have the same method names. It isn't necessary, though, and for clarity's sake it might even be better to keep the names distinct. Regardless, once again the compiler will help validate any refactoring.

(I know I stress the compiler bit a lot, but in my experience, once everything has the same name, and works through one or more layers of indirection, it becomes a pain to find out where and when something is actually going wrong.)

Anyway, this is how I would do it, as it allows for splitting up large chunks of code fairly quickly, in a controlled manner, without having to think too much. It provides a nice stepping stone for further tweaking. I guess that at some point the MyModelItem class should be renamed to MyModelItemMediator.

Good luck with your project.

Answer 3

If I understand your problem correctly, then would I not decide yet which design pattern to chose. I think that I have seen code like this several times before and the main problem in my point of view was always that change was build upon change build upon change. The class had lost is original purpose and was now serving multiple purposes, which were all not clearly defined and set. The result is a big class (or a big database, spaghetti code etc), which seems to be indispensable yet is a nightmare for maintenance.

The big class is the symptom of a process that is gone out of control. It is where you can see it happen, but my guess is that when this class has been recovered that a lot of other classes will be the first to redesign. If I am correct, then is there also a lot of data corruption, because in a lot of cases is the definition of the data unclear.

My advice would be go back to your customer, talk about the business processes, reorganize the project management of the application and try to find out if the application is still serving the business process well. It might be not - I have been in this type of situation several times in different organizations. If the business process is understood and the data model is converted in line with the new data model, then can you replace the application with a new design, which is much easier to create. The big class that now exists, does not have to be reorganized anymore, because its reason for existence is gone. It costs money, but the maintenance now costs also money. A good indication for redesign is if new features are not implemented anymore, because it has become too expensive or error prone to execute.

Answer 4

I will try to give you a different perspective of the situation you have. Please note that explanations are written in my own words for the simplicity's sake. However, terms mentioned are from the enterprise application architecture patterns.

You are designing the business logic of the application. So, MyModelItem must be some kind of a business entity. I would say it's the Active Record you have.

Active Record: business entity that can CRUD itself, and can manage the business logic related to itself.

The business logic contained in the Active Record has increased and has become hard to manage. That's very typical situation with Active Records. This is where you must switch from the Active Record pattern to the Data Mapper pattern.

Data Mapper: mechanism (typically a class) managing the mapping (typically between the entity and the data it translates from/to). It starts existing when the mapping concerns of the Active Record are so mature that they need to be put into the separate class. Mapping becomes a logic on its own.

So, here we came to the obvious solution: create a Data Mapper for the MyModelItem entity. Simplify the entity so that it does not handle the mapping of itself. Migrate the mapping management to the Data Mapper.

If the MyModelItem takes part in the inheritance, consider creating an abstract Data Mapper and concrete Data Mappers for each concrete class you want to map in a different way.

Several notes on how I would implement it:

• Make entity aware of a mapper.
• Mapper is a finder of the entity, so the application always starts from the mapper.
• Entity should expose the functionality that is natural to be found on it.
• And entity makes use of (abstract or concrete) mapper for doing the concrete things.

In general, you must model your application without the data in mind. Then, design mapper to manage the transformations from objects to the data and vice verca.

Now about validation

If the validation is the same in all the cases, then implement it in the entity, as that sounds natural to me. In most cases, this approach is sufficient.

If the validation differs and depends on something, abstract that something away and call the validation through the abstraction. One way (if it depends on the inheritance) would be to put the validation in the mapper, or have it in the same family of objects as mapper, created by the common Abstract Factory.