I know there has been many questions on the Model-View-Controller pattern, but this question is a follow up on a previous question I asked regarding the implementation of a download manager:
Get files downloaded asynchronously after double clicking on list item (C++)
One of the reviewers suggested that I implement (besides the threading aspect, which was the core of the question) the whole thing using an MVC pattern. As I had never done that before but was extremely interested in finally learning about this, I decided to address improvements to the code by first tackling the MVC implementation. I have been extremely impressed by the quality of the answers I received to my first question. So would like to progress in that way.
Now again I know they are many posts/questions about this topic, but I am particularly interested in the implementation I came up with.
- First, the issue with everything that is out there is that there are in fact, many different solutions provided to the problem, and most articles insist on the fact that there's no "definite" right way of implementing this pattern.
- As I have no experience, I have tried to learn how this is done by studying Chrome's source code. The code base is too complicated for someone of my level but I have been basing my observations on the very first version published so the complexity remains acceptable:
https://github.com/chromium/chromium/tree/3.0.195.25
I have been mostly looking at the ItemShelfDialog
which is a view that implements a table view and a table model. In the case of this implementation, this is would be a list of URLs (and the table model would essentially be a list of URLs).
I did my best to re-implement as well as I could a simplified version of that code (see below).
I did a diagram to show (if I got it right) the relations between the components. Whereas (in Google's implementation) the colors represent the files in which the different classes would be implemented (e.g. class TableModel and class ToableModelObserver are defined in the same file)
https://github.com/chromium/chromium/blob/3.0.195.25/app/table_model.h
Here is my understanding of a possible "correct" implementation:
The model is the central piece. It holds well... the model (the data).
The view (
TableView
) observes the model (I am the table model observer).The "controller" (not named controller in this particular example, but I understand the class
ShelfItemDialog
acts as the controller here) observes the view (I am the table view observer).The
ShelfItemDialog
(the wrapper view) holds the model and the view as member variables (it is itself a view so that it can be placed into the UI framework). When it is created, it creates the model creates the view, set itself (the controller) as the view's observer, and set the view as the model's observer.
Use cases:
- At creation, the model is loaded (
Reload()
), which causes the view to be "signaled" by the model throughOnModelChange()
. The view can now display the list of items from the table (a list of files, in my case). - The view receives events (double click) which causes the "controller" to be "signaled" through
OnDoubleClick
. The controller can then download the files (that was my original goal - design as an exercise, a program that downloads files).OnDoubleClick
not implemented in the code below.
Questions:
- Would like any general comments about the code itself of course (disclaimer: it's not because I based this on Google's code - also rather old - that mine is good. I may have incorrectly interpreted the original author's intent).
- More importantly, I want to understand if I got the pattern right (in terms of how it should be coded) and, if not, what I need to fix.
- Since the View has access to the model, can it queries the data from the model to display a list of say files (like in my case) or should it maintain its own internal
std::vector<std::pair<int, std::string>>
where theint
would be an index in the model's data, andstring
the name of the file as displayed in the view? - If a user deletes an item from the file list (say delete key), the view will receive this event. But then should the view call the model to say, "hey remove item at index X from your model data" or should the view call the controller (
RemoveItemByIndex(X)
)that in turn will call the model (RemoveItemByIndex(X)
), which in turn will call the view (update your view, the model changed). Looking at Google's code:
void TableView::OnKeyDown(unsigned short virtual_keycode) {
if (!ignore_listview_change_ && table_view_observer_) {
table_view_observer_->OnKeyDown(virtual_keycode);
}
}
It calls the observer (the controller), which would then process the key (which can be a del key). Is the latter then the correct way? (though in Google's impl, OnKeyDown()
is not overwritten - but irrelevant - it might as well had been). In general, should every event received by the view always be sent to the controller and never directly to the model? I know stupid question, since it would break the MVC pattern in the first place, yet, I am curious to see if this would sometimes be ok.
The code:
#include <vector>
#include <memory>
#include <string>
#include <iostream>
// MODELS
class TableModelObserver { // table_model_observer
public:
virtual void OnModelChanged() = 0;
//virtual void OnItemsChanged(int start, int length) = 0;
//virtual void OnItemsAdded(int start, int length) = 0;
//virtual void OnItemsRemoved(int start, int length) = 0;
};
class TableModel { // table_model
public:
struct Group {
std::wstring title;
int id;
};
typedef std::vector<Group> Groups;
virtual void SetObserver(TableModelObserver* observer) = 0;
};
class PossibleURLModel : public TableModel {
public:
PossibleURLModel() {
}
void Reload() {
// create some fake results
struct SomeTableResultType {
std::wstring filename;
std::wstring title;
};
std::vector<SomeTableResultType> results = {{L"path/foo.bin", L"foo"}, {L"path/bar.bin", L"bar"}};
results_.resize(results.size());
for (size_t i = 0; i < results.size(); ++i) {
results_[i].filename = results[i].filename;
results_[i].title = results[i].title;
results_[i].index = i;
}
if (observer_)
observer_->OnModelChanged();
}
virtual void SetObserver(TableModelObserver* observer) {
observer_ = observer;
}
struct Result {
std::wstring filename;
std::wstring title;
size_t index{0};
};
// Results we are showing
std::vector<Result> results_;
// Our observer
TableModelObserver* observer_;
};
// VIEWS
class View {
};
class TableViewObserver {
public:
//virtual void OnSelectionChanged() = 0;
virtual void OnDoubleClick() = 0;
};
class TableView : public View, TableModelObserver {
public:
TableView(TableModel* model)
: model_(model) {
model_->SetObserver(this); // the view is observing the model. If the model changes, table view shall be notified
}
void SetObserver(TableViewObserver* observer) {
table_view_observer_ = observer;
}
virtual void OnModelChanged() { std::cerr << "TableView::OnModelChanged()" << std::endl; }
TableModel* model_;
TableViewObserver* table_view_observer_{nullptr};
};
// CONTROLLER
class ShelfItemDialog : public View, TableViewObserver { // shelf_item_dialog
public:
ShelfItemDialog() {
url_table_model_.reset(new PossibleURLModel());
url_table_ = new TableView(url_table_model_.get());
url_table_->SetObserver(this);
}
~ShelfItemDialog() = default;
void Show() {
url_table_model_->Reload();
}
void OnDoubleClick() {
//int selection = url_table_->FirstSelectedRow();
//if (selection >= 0 && selection < url_table_model_->RowCount()) {
// OnSelectionChanged();
// PerformModelChange();
//}
}
TableView* url_table_;
std::unique_ptr<PossibleURLModel> url_table_model_;
};
int main() {
ShelfItemDialog a;
a.Show();
return 0;
}