gpospelov / qt-mvvm

Model View ViewModel framework for large Qt C++ applications
GNU General Public License v3.0
341 stars 86 forks source link

A question about architecture #338

Closed mherok closed 3 months ago

mherok commented 1 year ago

If I understand, this project often uses interfaces that are not usable, except for unit tests. Interfaces are used often as a class member, initialized by initialized list by one class implementation and there are no corresponding setters or constructors.

For example, I would like to use undo/redo functionality by using existing JSON serialization. In my model, I used more QVariant data types which JsonVariantConverter converters do not contain. Unfortunately, I can't find a way to extend the variant converter and use any injection. There exists JsonVariantConverterInterface but class implementation is injected like this:

JsonItemDataConverter::JsonItemDataConverter(accept_strategy_t to_json_accept,
                                             accept_strategy_t from_json_accept)
    : m_to_json_accept(to_json_accept)
    , m_from_json_accept(from_json_accept)
    , m_variant_converter(std::make_unique<JsonVariantConverter>())
{
}

A similar situation regarding JsonItemDataConverter initialization and other classes up in aggregation where some are additionally hidden by pimpl pattern.

One class to convert all current and future data types in variant can not exist. So, in this case, should I implement my whole serialization and the undo/redo stack or do I miss something?

gpospelov commented 1 year ago

You are pointing out actual problems in the architecture that do not allow real use as third party dependency. At the moment of the creation of this prototype, I wasn't paying enough attention to the extensibility and dependency injection.

You could create your own JsonVariantConverter as a decorator of the original one with extended functionality, but then you still need to hack it inside somehow. I have drifted away from this work and can't provide support, unfortunately.