7
\$\begingroup\$

While understanding the MVC pattern, I came across this particular article. I thought to implement it using the C++/QT library.

The intent over here is to implement/use MVC concepts in correct way. Awaiting the feedback/review comments from community.

enter image description here

enter image description here

model.h

#ifndef MODEL_H
#define MODEL_H
class Model {
public:
  Model();
  ~Model();
  double ConvertRuppesIntoDollor(double rs);
  double GetDollorValue()const;
  double GetRuppeValue() const;
  void clear();
private:
  double r;
  double d;
};
#endif // MODEL_H

model.cpp

#include "model.h"

Model::Model(): r(),d() {}
Model::~Model(){}
double Model::ConvertRuppesIntoDollor(double rs) {
  r = rs;
  d = r/64.0;
  return d;
}
double Model::GetDollorValue() const { return d;}
double Model::GetRuppeValue()  const { return r;}
void Model::clear() {
  r = 0;
  d = 0;
}

view.h

#ifndef VIEW_H
#define VIEW_H
#include <QWidget>
#include <QString>
// Forward Declaration
class QPushButton;
class QLineEdit;
class QHBoxLayout;
class Controller;

class View : public QWidget {
  Q_OBJECT
public:
  explicit View(QWidget *parent = NULL, QString name = "MVC");
  virtual ~View();
  void setController(Controller* c);
  QString getDollor();
  QString getRuppes();
  void    setDollor(QString d);
  void    setRuppes(QString r);
public slots:
  void ConvertButtonClicked();
  void ClearButtonClicked();
private:
  QPushButton* press;
  QPushButton* clear;
  QLineEdit*   dollorinfo;
  QLineEdit*   ruppesinfo;
  QHBoxLayout* hlayout;
  Controller*  controller;
  QString      appName;
  QString      dollor;
  QString      ruppes;
};
#endif // VIEW_H

view.cpp

#include "view.h"
#include "controller.h"
#include <QPushButton>
#include <QLineEdit>
#include <QHBoxLayout>
#include <iostream>

View::View(QWidget *parent, QString name)
  : QWidget(parent), appName(name) {
  this->setWindowTitle(appName);
  //Create a horizontal container widgets.
  hlayout = new QHBoxLayout(this);
  hlayout->setSpacing(1);
  ruppesinfo = new QLineEdit(this);
  ruppesinfo->setPlaceholderText("RuppesInfo");
  dollorinfo = new QLineEdit(this);
  dollorinfo->setPlaceholderText("DollorInfo");
  QString buttonname = "Convert";
  press = new QPushButton(buttonname, this);
  QString clearname = "Clear";
  clear = new QPushButton(clearname, this);

  //Now add all child widgets inside parent one
  hlayout->addWidget(ruppesinfo);
  hlayout->addWidget(dollorinfo);
  hlayout->addWidget(press);
  hlayout->addWidget(clear);
  //Connect the appropriate signal
  connect(press, SIGNAL(clicked(bool)), this, SLOT(ConvertButtonClicked()));
  connect(clear, SIGNAL(clicked(bool)), this, SLOT(ClearButtonClicked()));
}
View::~View(){}
void View::setController(Controller* c) {controller = c;}
QString View::getDollor()        {return dollorinfo->text();}
QString View::getRuppes()        {return ruppesinfo->text();}
void View::setDollor(QString d)  {dollorinfo->setText(d);}
void View::setRuppes(QString r)  {ruppesinfo->setText(r);}
void View::ConvertButtonClicked(){controller->OnConvertButtonClicked(this);}
void View::ClearButtonClicked()  {controller->OnClearButtonClicked(this);}

controller.h

#ifndef CONTROLLER_H
#define CONTROLLER_H
//Forward Declaration
class Model;
class View;
class QString;

class Controller {
public:
  Controller(Model* m);
  virtual ~Controller();
  void OnConvertButtonClicked(View* v);
  void OnClearButtonClicked(View* v);
private:
  Model* model;
};
#endif // CONTROLLER_H

controller.cpp

#include "controller.h"
#include "model.h"
#include "view.h"
#include <QString>

Controller::Controller(Model* m):model(m){}
Controller::~Controller(){}
void Controller::OnConvertButtonClicked(View* v) {
  QString ruppes = v->getRuppes();
  model->ConvertRuppesIntoDollor(ruppes.toFloat());
  QString ds = QString::number(model->GetDollorValue());
  v->setDollor(ds);
}
void Controller::OnClearButtonClicked(View* v) {
  v->setDollor(QString());
  v->setRuppes(QString());
  model->clear();
}

client.cpp

#include <QApplication>
#include "view.h"
#include "model.h"
#include "controller.h"
int main(int argc, char *argv[]) {
    QApplication app(argc, argv);
    Model model;
    View window;
    Controller ctrl(&model);
    window.setController(&ctrl);
    window.show();
    return app.exec();
}
\$\endgroup\$

4 Answers 4

13
\$\begingroup\$

The typical MVC pattern has a data flow like this:

 |--->Model  ->   View   ->   Controller  ----|
 |                                            |
 |--------------------------------------------|

So data flows from the Model to the View (When State is updated)
So data flows from the View to the Controller (When the user interacts)
So data flows from the Controller to the Model.

Usually how this works.

  1. User updates the view.
  2. Any control that is interested in the view has registered itself as an observer and thus gets updates notification from the view when the content is changed.
  3. The control processes the notification and changes the model state.
  4. Any view that is interested in a model has registered itself as an observer and has gets update notifications when the model changes and can revise the content of the view.

Note: I am using the generic term Observe here. QT implements this as signals and slots. But that is just an implementation detail.

In a lot of situations there is only one controller per view. So a lot frameworks bundle the View/Controller into a single entity to reduce overhead (but there is no harm in splitting them out).

On the other hand your flow is more like this:

                   (view pulls from model)
      Model       View   ->   Controller  ----|
       ^            ^                         |
       |            |                         |
       ---------------------------------------|

The trouble with this design is that; if you write more code that interacts with the model then the view does not get updated when you change the model.

For example: Say you had ten more currencies.
They are all using the same model. You just have 10 different views on the model to display it in 10 different currencies. Now when you update the model via one control. Only the view associated with that control updates (As it does not know about the other views). What you really want is all the views to register interest in the model. When its state changes they should be notified and update their displays appropriately.

Currently when you add more views the control has to know about each view that could be affected and manually force it to update. This is very error prone.

Now that I have read the article you reference. I see why your code works the way it does. You are following the pattern described. I would hesitate to say that the author if that article is wrong.

See: https://en.wikipedia.org/wiki/Model%E2%80%93view%E2%80%93controller

As I mentioned above if each control updates the model independently then only the view(s) associated with that particular control will update. Any other V/C that also use the model are not going to update. So every new view that you add you have to go and find every control that updates the model and tell it about the new view so it can tell that view to update as well (this is error prone).

The original pattern is much better. If I add a new view I just register the view as an observer on the model. When the model is updated (by a control) my view is notified and will potentially change its view to reflect the change in the model.

\$\endgroup\$
2
  • \$\begingroup\$ Isn't the controller sort of a bad idea with qt? I think controller does't really belong in Qt because it has signals and slot mechanism. If I were to write this demo, I would have eliminated the controller part. The controller job is mainly to connect view to the model but it seems redundant with signals and slot does that. In my view signals and slots replace the controller..is that fair assessment? \$\endgroup\$
    – zadane
    Commented Oct 27, 2015 at 21:16
  • \$\begingroup\$ @zadane: You are thinking incorrectly. MVC is a pattern. Different frameworks implement the details of the pattern but the pattern remains. In QT they have decided to combine the functionality of view and the controller into a single object (a valid implementation but it has tradeoffs in flexibility; that designers of QT decided that it was worth these trade offs).doc.qt.io/qt-4.8/model-view-programming.html \$\endgroup\$ Commented Oct 27, 2015 at 21:25
4
\$\begingroup\$

I have used a similar approach to implement MVC as you have Mantosh. I would however use references/pointers for both the view and model objects within the controller-class. This would then enable you to directly use the signal-slot mechanism and not need to set a controller in the viewer. This would also imply that the view and model are fully independent from each other, which I think is the greatest advantage of using MVC.

As far as I understand the implementation Loki suggests means that the view needs to understand how the model works (in order to understand the notifications) which implies that the view is dependent on the model? Is that a correct interpretation?

I prefer something like Apple suggests: https://developer.apple.com/library/ios/documentation/General/Conceptual/DevPedia-CocoaCore/MVC.html

\$\endgroup\$
3
\$\begingroup\$

Intro

You have a good review of your structure and its relation to the MVC pattern from Martin York, so I won't reiterate those points. Instead, I'll look at a lower level for some improvements to the code. They are common to much Qt code that I see, so might provide some general learning.

Spelling

Firstly, please use conventional spellings of words like "dollar" and "rupee" - it makes reading code much easier!

Includes

We include <iostream> but never use it.

Unused member functions

Model has a destructor that's identical to the implicit destructor, so it can be removed. Although it's not relevant here, an explicitly declared destructor can inhibit certain optimisations that compilers cannot perform on non-trivial classes, so don't get in the habit of writing unnecessary destructors.

Similarly, there's no need to write virtual ~View() - the base class has a virtual destructor, and ~View() is empty, so just remove it.

The virtual destructor for Controller can be written = default (although I question the need for this, unless you really intend it to be used as a base class, so we could probably remove it entirely).

Unnecessary member objects

Model doesn't need to store both d and r, given that one is computed from the other. On the other hand, it would make sense for the exchange rate to be a settable variable, rather than baked into the code.

View keeps pointers to all its widgets, but doesn't use most of them. The layout and the buttons are used only in the constructor, so they can just be local variables (remember that passing this as the parent object will cause them to be owned by the view, and so be destructed with it).

The dollor and ruppes members are never initialised or used, so can be immediately removed.

View shouldn't need to know about Controller. Instead, it's better to give View the appropriate signals and slots so that the application that uses it can connect it to any controller it chooses.


I think that part of your struggle is that the Qt classes such as QLineEdit are already views and controllers, and to use them in your application is usually just a matter of connecting them appropriately to the model which you write. I'm going to re-write the application from scratch, showing a model that's designed to work with the Qt widgets, without being tightly coupled as this code is.

I'll start with the interface for our model:

#include <QObject>

class Converter : public QObject
{
    Q_OBJECT

public:
    explicit Converter(QObject *parent = nullptr)
        : QObject{parent},
          value{},
          rate{}
    {}

public slots:
    void setInputAmount(double v)
    {
        if (value == v) return;
        emit outputAmountChanged((value = v) * rate);
    }
    void setExchangeRate(double r)
    {
        if (rate == r) return;
        emit outputAmountChanged(value * (rate = r));
    }

signals:
    void outputAmountChanged(double amount);

private:
    double value;
    double rate;
};

All this class knows about are the domain objects, namely amount and exchange rate. The main thing I've changed is that it's now a QObject, so it can be connected to other signals and slots.

Then we create our UI; again the key to this is that we expose our Controller as signals, and our View interface as slots (actually, here I use a signal, as that's how signals are forwarded in Qt):

#include <QDoubleSpinBox>
#include <QHBoxLayout>
#include <QLabel>
#include <QWidget>

class Converter_UI : public QWidget
{
    Q_OBJECT

public:
    explicit Converter_UI(QWidget *parent = nullptr)
        : QWidget{parent}
    {
        // Using Qt spin box as a controller
        auto *const dollars = new QDoubleSpinBox{this};
        dollars->setDecimals(2);
        dollars->setPrefix("$");
        dollars->setMaximum(1e6);
        connect(dollars, qOverload<double>(&QDoubleSpinBox::valueChanged),
                this, &Converter_UI::dollarsChanged);

        // Same for rate
        auto *const rate = new QDoubleSpinBox{this};
        rate->setDecimals(4);
        rate->setMinimum(0);
        rate->setMaximum(1e3);
        connect(rate, qOverload<double>(&QDoubleSpinBox::valueChanged),
                this, &Converter_UI::rateChanged);

        // Finally, a label as a view
        auto *const rupees = new QLabel{this};
        rupees->setMinimumWidth(80);
        connect(this, &Converter_UI::setRupees,
                rupees, [rupees](double d){ rupees->setText("₨."+QString::number(d, 'f', 2)); });

        // Add them as child widgets
        auto *const hlayout = new QHBoxLayout(this);
        hlayout->setSpacing(1);

        hlayout->addWidget(dollars);
        hlayout->addWidget(rate);
        hlayout->addWidget(rupees);
    }

signals:
    // this is our View interface
    void setRupees(double rupees);

signals:
    // this is our Controller
    void dollarsChanged(double dollars);
    void rateChanged(double rate);
};

Note that this UI knows nothing about the controller we're going to connect it to.

Here's where we connect the UI View/Controller to the Model:

#include <QApplication>
int main(int argc, char *argv[])
{
    QApplication app(argc, argv);
    Converter converter;
    Converter_UI ui;

    // connect views first
    QObject::connect(&converter, &Converter::outputAmountChanged,
                     &ui, &Converter_UI::setRupees);
    // connect controllers
    QObject::connect(&ui, &Converter_UI::dollarsChanged,
                     &converter, &Converter::setInputAmount);
    QObject::connect(&ui, &Converter_UI::rateChanged,
                     &converter, &Converter::setExchangeRate);

    ui.show();
    return app.exec();
}
\$\endgroup\$
-1
\$\begingroup\$

You can also consider going more for a QML-driven apps instead of Qt C++. Qt already has Model-View separation built-in for both QML and C++.

You can find an interesting guide here, which outlines the advantages of QML and how to structure apps in Flux-like (React Native) way

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Whilst that's certainly an alternative, a Code Review really needs to explain why the current approach is unsuitable, and how the proposed approach improves on it. \$\endgroup\$ Commented Jan 31, 2019 at 16:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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