From b3ddeda86d54c41f4c4e5ea290dc63c539f9692b Mon Sep 17 00:00:00 2001
From: Gemma Guest <gemma.guest@stfc.ac.uk>
Date: Thu, 27 Jun 2019 10:52:22 +0100
Subject: [PATCH] Fix a crash when the help button is clicked
- This was to do with the way the presenter is created as an optional. It doesn't need to be an optional here - a unique pointer is better and makes ownership clearer.
- This commit also includes some tidying of the MainWindowSubscriber which was being used incorrectly.
- It also includes replacing some calls to m_presenter with calls to m_notifyee. They are the same object but we should use m_notifyee to be more in keeping with MVP. There is one remaining use of m_presenter in closeEvent but it is beyond the scope of this PR to fix that.
Re #23027
---
.../GUI/MainWindow/IMainWindowPresenter.h | 2 +-
.../ISISReflectometry/GUI/MainWindow/IMainWindowView.h | 6 +++---
.../GUI/MainWindow/MainWindowPresenter.h | 8 +++++++-
.../ISISReflectometry/GUI/MainWindow/MainWindowView.cpp | 9 +++++----
.../ISISReflectometry/GUI/MainWindow/MainWindowView.h | 7 +++++--
5 files changed, 21 insertions(+), 11 deletions(-)
diff --git a/qt/scientific_interfaces/ISISReflectometry/GUI/MainWindow/IMainWindowPresenter.h b/qt/scientific_interfaces/ISISReflectometry/GUI/MainWindow/IMainWindowPresenter.h
index f5586a4a088..7dfc97c2f8a 100644
--- a/qt/scientific_interfaces/ISISReflectometry/GUI/MainWindow/IMainWindowPresenter.h
+++ b/qt/scientific_interfaces/ISISReflectometry/GUI/MainWindow/IMainWindowPresenter.h
@@ -19,7 +19,7 @@ IMainWindowPresenter is the interface defining the functions that the main
window presenter needs to implement. This interface is used by tab presenters to
request information from other tabs.
*/
-class IMainWindowPresenter : public MainWindowSubscriber {
+class IMainWindowPresenter {
public:
virtual bool isProcessing() const = 0;
virtual ~IMainWindowPresenter() = default;
diff --git a/qt/scientific_interfaces/ISISReflectometry/GUI/MainWindow/IMainWindowView.h b/qt/scientific_interfaces/ISISReflectometry/GUI/MainWindow/IMainWindowView.h
index b4b190a8dd3..59c16a5fbae 100644
--- a/qt/scientific_interfaces/ISISReflectometry/GUI/MainWindow/IMainWindowView.h
+++ b/qt/scientific_interfaces/ISISReflectometry/GUI/MainWindow/IMainWindowView.h
@@ -23,9 +23,9 @@ the help button.
*/
class MainWindowSubscriber {
public:
- virtual void notifyHelpPressed(){};
- virtual void notifyNewBatchRequested(){};
- virtual void notifyCloseBatchRequested(int){};
+ virtual void notifyHelpPressed() = 0;
+ virtual void notifyNewBatchRequested() = 0;
+ virtual void notifyCloseBatchRequested(int) = 0;
virtual ~MainWindowSubscriber() = default;
};
diff --git a/qt/scientific_interfaces/ISISReflectometry/GUI/MainWindow/MainWindowPresenter.h b/qt/scientific_interfaces/ISISReflectometry/GUI/MainWindow/MainWindowPresenter.h
index 396d67582c3..5ad86af2c72 100644
--- a/qt/scientific_interfaces/ISISReflectometry/GUI/MainWindow/MainWindowPresenter.h
+++ b/qt/scientific_interfaces/ISISReflectometry/GUI/MainWindow/MainWindowPresenter.h
@@ -10,6 +10,7 @@
#include "Common/DllConfig.h"
#include "GUI/Batch/BatchPresenterFactory.h"
#include "IMainWindowPresenter.h"
+#include "IMainWindowView.h"
#include <memory>
namespace MantidQt {
@@ -23,12 +24,17 @@ MainWindowPresenter is the concrete main window presenter implementing the
functionality defined by the interface IMainWindowPresenter.
*/
class MANTIDQT_ISISREFLECTOMETRY_DLL MainWindowPresenter
- : public IMainWindowPresenter {
+ : public MainWindowSubscriber,
+ public IMainWindowPresenter {
public:
/// Constructor
MainWindowPresenter(IMainWindowView *view,
BatchPresenterFactory batchPresenterFactory);
+
+ // IMainWindowPresenter overrides
bool isProcessing() const override;
+
+ // MainWindowSubscriber overrides
void notifyHelpPressed() override;
void notifyNewBatchRequested() override;
void notifyCloseBatchRequested(int batchIndex) override;
diff --git a/qt/scientific_interfaces/ISISReflectometry/GUI/MainWindow/MainWindowView.cpp b/qt/scientific_interfaces/ISISReflectometry/GUI/MainWindow/MainWindowView.cpp
index 432bd9988fe..391a5dacfab 100644
--- a/qt/scientific_interfaces/ISISReflectometry/GUI/MainWindow/MainWindowView.cpp
+++ b/qt/scientific_interfaces/ISISReflectometry/GUI/MainWindow/MainWindowView.cpp
@@ -90,10 +90,11 @@ void MainWindowView::initLayout() {
std::move(makeSaveSettingsPresenter));
// Create the presenter
- m_presenter = MainWindowPresenter(this, std::move(makeBatchPresenter));
+ m_presenter = std::make_unique<MainWindowPresenter>(
+ this, std::move(makeBatchPresenter));
- m_presenter.get().notifyNewBatchRequested();
- m_presenter.get().notifyNewBatchRequested();
+ m_notifyee->notifyNewBatchRequested();
+ m_notifyee->notifyNewBatchRequested();
}
void MainWindowView::onTabCloseRequested(int tabIndex) {
@@ -127,7 +128,7 @@ Handles attempt to close main window
*/
void MainWindowView::closeEvent(QCloseEvent *event) {
// Close only if reduction has been paused
- if (!m_presenter.get().isProcessing()) {
+ if (!m_presenter->isProcessing()) {
event->accept();
} else {
event->ignore();
diff --git a/qt/scientific_interfaces/ISISReflectometry/GUI/MainWindow/MainWindowView.h b/qt/scientific_interfaces/ISISReflectometry/GUI/MainWindow/MainWindowView.h
index 0fe75e61483..216e183c023 100644
--- a/qt/scientific_interfaces/ISISReflectometry/GUI/MainWindow/MainWindowView.h
+++ b/qt/scientific_interfaces/ISISReflectometry/GUI/MainWindow/MainWindowView.h
@@ -63,8 +63,11 @@ private:
/// Interface definition with widgets for the main interface window
Ui::MainWindowWidget m_ui;
MainWindowSubscriber *m_notifyee;
- /// The presenter handling this view
- boost::optional<MainWindowPresenter> m_presenter;
+ /// The presenter handling this view. It is not normal in MVP for a view to
+ /// have ownership of its presenter, but due to the way interfaces get
+ /// instantiated this is currently necessary for MainWindowView. Direct use
+ /// of m_presenter should be avoided - use m_notifyee instead.
+ std::unique_ptr<MainWindowPresenter> m_presenter;
std::vector<IBatchView *> m_batchViews;
};
} // namespace CustomInterfaces
--
GitLab