Skip to content

Commit

Permalink
refs #7281. Refactor factory and view interfaces.
Browse files Browse the repository at this point in the history
.White box testing provided by the mocking framework has been a lifesaver here!
  • Loading branch information
OwenArnold committed Jun 12, 2013
1 parent 04c2fb5 commit 0874d4d
Show file tree
Hide file tree
Showing 13 changed files with 47 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ namespace SliceViewer
/// Change background colour
virtual void changeBackgroundColour(const QColor);
/// Get a bounding box for this peak.
virtual PeakBoundingBox getBoundingBox() const;
virtual PeakBoundingBox getBoundingBox(const int peakIndex) const;
/// Changes the size of the overlay to be the requested fraction of the current view width.
virtual void changeOccupancyInView(const double fraction);
/// Changes the size of the overlay to be the requested fraction of the view depth.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ namespace MantidQt
public:
PeakOverlayMultiCrossFactory(boost::shared_ptr<Mantid::API::MDGeometry> mdWS, PeakTransform_const_sptr transform, Mantid::API::IPeaksWorkspace_sptr peaksWS, QwtPlot * plot, QWidget * parent, const size_t colourNumber=0);
virtual ~PeakOverlayMultiCrossFactory();
virtual boost::shared_ptr<PeakOverlayView> createView(const int peakIndex, PeakTransform_const_sptr transform) const;
virtual boost::shared_ptr<PeakOverlayView> createView(PeakTransform_const_sptr transform) const;
virtual int FOM() const;
private:
/// Peaks workspace.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ namespace SliceViewer
/// Show the background radius
virtual void showBackgroundRadius(const bool show);
/// Get a bounding box for this peak.
virtual PeakBoundingBox getBoundingBox() const;
virtual PeakBoundingBox getBoundingBox(const int peakIndex) const;
/// Changes the size of the overlay to be the requested fraction of the current view width.
virtual void changeOccupancyInView(const double fraction);
/// Changes the size of the overlay to be the requested fraction of the view depth.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ namespace MantidQt
public:
PeakOverlayMultiSphereFactory(Mantid::API::IPeaksWorkspace_sptr peaksWS, QwtPlot * plot, QWidget * parent, const size_t colourNumber=0);
virtual ~PeakOverlayMultiSphereFactory();
virtual boost::shared_ptr<PeakOverlayView> createView(const int peakIndex, PeakTransform_const_sptr transform) const;
virtual boost::shared_ptr<PeakOverlayView> createView(PeakTransform_const_sptr transform) const;
virtual int FOM() const;
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ namespace MantidQt
/// Changes the size of the overlay to be the requested fraction of the view depth.
virtual void changeOccupancyIntoView(const double fraction) = 0;
/// Get a bounding box around the peak in windows coordinates.
virtual PeakBoundingBox getBoundingBox() const = 0;
virtual PeakBoundingBox getBoundingBox(const int peakIndex) const = 0;
/// Get the peak size (width/2 as a fraction of total width) on projection
virtual double getOccupancyInView() const = 0;
/// Get the peaks size into the projection (effective radius as a fraction of z range)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ namespace MantidQt
{
public:
/// Create a peak view from the index of a peak in the peaks workspace
virtual boost::shared_ptr<PeakOverlayView> createView(const int peakIndex, PeakTransform_const_sptr transform) const = 0;
virtual boost::shared_ptr<PeakOverlayView> createView(PeakTransform_const_sptr transform) const = 0;
/// Destructor
virtual ~PeakOverlayViewFactory()
{
Expand Down
12 changes: 7 additions & 5 deletions Code/Mantid/MantidQt/SliceViewer/src/ConcretePeaksPresenter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ namespace MantidQt
*/
void ConcretePeaksPresenter::produceViews()
{
m_viewPeaks = m_viewFactory->createView(0, m_transform);
m_viewPeaks = m_viewFactory->createView(m_transform);
}

/**
Expand Down Expand Up @@ -284,16 +284,19 @@ namespace MantidQt
}
m_viewPeaks->updateView();
}

}

/**
@param peakIndex: index into contained peaks workspace.
@return the bounding box corresponding to the peakIndex.
*/
PeakBoundingBox ConcretePeaksPresenter::getBoundingBox(const int) const
PeakBoundingBox ConcretePeaksPresenter::getBoundingBox(const int peakIndex) const
{
throw std::runtime_error("Not implemented yet");
if(peakIndex < 0 || peakIndex > m_peaksWS->rowCount())
{
throw std::out_of_range("Index given to ConcretePeaksPresenter::getBoundingBox() is out of range.");
}
return m_viewPeaks->getBoundingBox(peakIndex);
}

void ConcretePeaksPresenter::sortPeaksWorkspace(const std::string& byColumnName,
Expand Down Expand Up @@ -326,7 +329,6 @@ namespace MantidQt
{
m_viewPeaks->changeOccupancyInView(fraction);
m_viewPeaks->updateView();

}

void ConcretePeaksPresenter::setPeakSizeIntoProjection(const double fraction)
Expand Down
16 changes: 5 additions & 11 deletions Code/Mantid/MantidQt/SliceViewer/src/PeakOverlayMultiCross.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,12 @@ namespace MantidQt
}

/**
@param peakIndex: Index of the peak to fetch the bounding box for.
@return bounding box for peak in windows coordinates.
*/
PeakBoundingBox PeakOverlayMultiCross::getBoundingBox() const
PeakBoundingBox PeakOverlayMultiCross::getBoundingBox(const int peakIndex) const
{
return m_physicalPeaks[0]->getBoundingBox(); // Hack
return m_physicalPeaks[peakIndex]->getBoundingBox();
}

/**
Expand All @@ -158,7 +159,6 @@ namespace MantidQt
{
for(int i = 0; i < m_physicalPeaks.size(); ++i)
{

m_physicalPeaks[i]->setOccupancyInView(fraction);
}
}
Expand All @@ -177,18 +177,12 @@ namespace MantidQt

double PeakOverlayMultiCross::getOccupancyInView() const
{
for(int i = 0; i < m_physicalPeaks.size(); ++i)
{
return m_physicalPeaks[i]->getOccupancyInView();
}
return m_physicalPeaks[0]->getOccupancyInView();
}

double PeakOverlayMultiCross::getOccupancyIntoView() const
{
for(int i = 0; i < m_physicalPeaks.size(); ++i)
{
return m_physicalPeaks[i]->getOccupancyIntoView();
}
return m_physicalPeaks[0]->getOccupancyIntoView();
}

bool PeakOverlayMultiCross::positionOnly() const
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ namespace MantidQt
{
}

boost::shared_ptr<PeakOverlayView> PeakOverlayMultiCrossFactory::createView(const int, PeakTransform_const_sptr transform) const
boost::shared_ptr<PeakOverlayView> PeakOverlayMultiCrossFactory::createView(PeakTransform_const_sptr transform) const
{
// Construct all physical peaks
VecPhysicalCrossPeak physicalPeaks(m_peaksWS->rowCount());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,12 @@ namespace MantidQt
}

/**
@param peakIndex: Index of the peak to fetch the bounding box for.
@return bounding box for peak in windows coordinates.
*/
PeakBoundingBox PeakOverlayMultiSphere::getBoundingBox() const
PeakBoundingBox PeakOverlayMultiSphere::getBoundingBox(const int peakIndex) const
{
//return m_physicalPeak.getBoundingBox();
throw std::runtime_error("PeakOverlayMultiSphere::getBoundingBox not implemented yet");
return m_physicalPeaks[peakIndex]->getBoundingBox();
}

void PeakOverlayMultiSphere::changeOccupancyInView(const double)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ namespace MantidQt
}
}

boost::shared_ptr<PeakOverlayView> PeakOverlayMultiSphereFactory::createView(const int, PeakTransform_const_sptr transform) const
boost::shared_ptr<PeakOverlayView> PeakOverlayMultiSphereFactory::createView(PeakTransform_const_sptr transform) const
{
// Construct all physical peaks
VecPhysicalSphericalPeak physicalPeaks(m_peaksWS->rowCount());
Expand Down
44 changes: 22 additions & 22 deletions Code/Mantid/MantidQt/SliceViewer/test/ConcretePeaksPresenterTest.h
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ class ConcretePeaksPresenterTest : public CxxTest::TestSuite
auto pMockViewFactory = new MockPeakOverlayFactory;

PeakOverlayViewFactory_sptr mockViewFactory = PeakOverlayViewFactory_sptr(pMockViewFactory);
EXPECT_CALL(*pMockViewFactory, createView(_,_)).WillRepeatedly(Return(mockView));
EXPECT_CALL(*pMockViewFactory, createView(_)).WillRepeatedly(Return(mockView));
EXPECT_CALL(*pMockViewFactory, getPlotXLabel()).WillRepeatedly(Return("H"));
EXPECT_CALL(*pMockViewFactory, getPlotYLabel()).WillRepeatedly(Return("K"));

Expand Down Expand Up @@ -195,7 +195,7 @@ class ConcretePeaksPresenterTest : public CxxTest::TestSuite
void test_construction()
{
// Expected number of peaks to create
const size_t expectedNumberPeaks = 3;
const size_t expectedNumberPeaks = 1;

// Peaks workspace IS INTEGRATED.
IPeaksWorkspace_sptr peaksWS = createPeaksWorkspace(expectedNumberPeaks);
Expand All @@ -210,7 +210,7 @@ class ConcretePeaksPresenterTest : public CxxTest::TestSuite
// Mock View Factory for integrated peaks. We expect that this will never be used.
auto pMockViewFactory = new MockPeakOverlayFactory;
PeakOverlayViewFactory_sptr mockViewFactory(pMockViewFactory);
EXPECT_CALL(*pMockViewFactory, createView(_,_)).Times(expectedNumberPeaks).WillRepeatedly(Return(mockView));
EXPECT_CALL(*pMockViewFactory, createView(_)).Times(1).WillRepeatedly(Return(mockView)); // Create a single widget/view for all peaks
EXPECT_CALL(*pMockViewFactory, getPlotXLabel()).WillOnce(Return("H"));
EXPECT_CALL(*pMockViewFactory, getPlotYLabel()).WillOnce(Return("K"));

Expand Down Expand Up @@ -248,10 +248,10 @@ class ConcretePeaksPresenterTest : public CxxTest::TestSuite

// Create a mock view object that will be returned by the mock factory.
auto pMockView = new NiceMock<MockPeakOverlayView>;
EXPECT_CALL(*pMockView, updateView()).Times(expectedNumberPeaks);
EXPECT_CALL(*pMockView, updateView()).Times(1); // Single view, for this presenter, will only update once.
auto mockView = boost::shared_ptr<NiceMock<MockPeakOverlayView> >(pMockView);

EXPECT_CALL(*pMockViewFactory, createView(_,_)).WillRepeatedly(Return(mockView));
EXPECT_CALL(*pMockViewFactory, createView(_)).WillRepeatedly(Return(mockView));
EXPECT_CALL(*pMockViewFactory, getPlotXLabel()).WillOnce(Return("H"));
EXPECT_CALL(*pMockViewFactory, getPlotYLabel()).WillOnce(Return("K"));

Expand Down Expand Up @@ -290,10 +290,10 @@ class ConcretePeaksPresenterTest : public CxxTest::TestSuite

// Create a mock view object that will be returned by the mock factory.
auto pMockView = new NiceMock<MockPeakOverlayView>;
EXPECT_CALL(*pMockView, setSlicePoint(slicePoint)).Times(expectedNumberPeaks);
EXPECT_CALL(*pMockView, setSlicePoint(slicePoint)).Times(1); // Only one widget for this presenter
auto mockView = boost::shared_ptr<NiceMock<MockPeakOverlayView> >(pMockView);

EXPECT_CALL(*mockViewFactory, createView(_,_)).WillRepeatedly(Return(mockView));
EXPECT_CALL(*mockViewFactory, createView(_)).WillRepeatedly(Return(mockView));
EXPECT_CALL(*mockViewFactory, getPlotXLabel()).WillOnce(Return("H"));
EXPECT_CALL(*mockViewFactory, getPlotYLabel()).WillOnce(Return("K"));

Expand Down Expand Up @@ -335,7 +335,7 @@ class ConcretePeaksPresenterTest : public CxxTest::TestSuite
EXPECT_CALL(*pMockView, hideView()).Times(expectedNumberPeaks);
auto mockView = boost::shared_ptr<NiceMock<MockPeakOverlayView> >(pMockView);

EXPECT_CALL(*mockViewFactory, createView(_,_)).WillRepeatedly(Return(mockView));
EXPECT_CALL(*mockViewFactory, createView(_)).WillRepeatedly(Return(mockView));
EXPECT_CALL(*mockViewFactory, getPlotXLabel()).WillOnce(Return("H"));
EXPECT_CALL(*mockViewFactory, getPlotYLabel()).WillOnce(Return("K"));

Expand Down Expand Up @@ -375,7 +375,7 @@ class ConcretePeaksPresenterTest : public CxxTest::TestSuite
EXPECT_CALL(*pMockView, hideView()).Times(expectedNumberPeaks); // This will be called automatically because the presenter won't be able to map Qx (below).
auto mockView = boost::shared_ptr<NiceMock<MockPeakOverlayView> >(pMockView);

EXPECT_CALL(*mockViewFactory, createView(_,_)).WillRepeatedly(Return(mockView));
EXPECT_CALL(*mockViewFactory, createView(_)).WillRepeatedly(Return(mockView));
EXPECT_CALL(*mockViewFactory, getPlotXLabel()).WillOnce(Return("Qx")); // Not either H, K or L
EXPECT_CALL(*mockViewFactory, getPlotYLabel()).WillOnce(Return("K"));
// Create an input MODEL Peaks workspace (INTEGRATED)
Expand Down Expand Up @@ -407,12 +407,12 @@ class ConcretePeaksPresenterTest : public CxxTest::TestSuite
// Create a mock view object/product that will be returned by the mock factory.
auto pMockView = new NiceMock<MockPeakOverlayView>;
auto mockView = boost::shared_ptr<NiceMock<MockPeakOverlayView> >(pMockView);
EXPECT_CALL(*pMockView, changeForegroundColour(colourToChangeTo)).Times(nPeaks); // Expect that the foreground colour of each widget will be changed.
EXPECT_CALL(*pMockView, updateView()).Times(nPeaks); // Expect that the background colour of each widget will be changed.
EXPECT_CALL(*pMockView, changeForegroundColour(colourToChangeTo)).Times(1); // Expect that the foreground colour will be changed.
EXPECT_CALL(*pMockView, updateView()).Times(1); // Only one view for this presenter.
// Create a widget factory mock
auto pMockViewFactory = new MockPeakOverlayFactory;
PeakOverlayViewFactory_sptr mockViewFactory = PeakOverlayViewFactory_sptr(pMockViewFactory);
EXPECT_CALL(*pMockViewFactory, createView(_,_)).WillRepeatedly(Return(mockView));
EXPECT_CALL(*pMockViewFactory, createView(_)).WillRepeatedly(Return(mockView));
EXPECT_CALL(*pMockViewFactory, getPlotXLabel()).WillRepeatedly(Return("H"));
EXPECT_CALL(*pMockViewFactory, getPlotYLabel()).WillRepeatedly(Return("K"));

Expand All @@ -433,12 +433,12 @@ class ConcretePeaksPresenterTest : public CxxTest::TestSuite
// Create a mock view object/product that will be returned by the mock factory.
auto pMockView = new NiceMock<MockPeakOverlayView>;
auto mockView = boost::shared_ptr<NiceMock<MockPeakOverlayView> >(pMockView);
EXPECT_CALL(*pMockView, changeBackgroundColour(colourToChangeTo)).Times(nPeaks); // Expect that the background colour on each widget will be changed.
EXPECT_CALL(*pMockView, updateView()).Times(nPeaks); // Expect that each widget will be updated.
EXPECT_CALL(*pMockView, changeBackgroundColour(colourToChangeTo)).Times(1); // Expect that the background colour will be changed.
EXPECT_CALL(*pMockView, updateView()).Times(1); // Expect that each widget will be updated.
// Create a widget factory mock
auto pMockViewFactory = new MockPeakOverlayFactory;
PeakOverlayViewFactory_sptr mockViewFactory = PeakOverlayViewFactory_sptr(pMockViewFactory);
EXPECT_CALL(*pMockViewFactory, createView(_,_)).WillRepeatedly(Return(mockView));
EXPECT_CALL(*pMockViewFactory, createView(_)).WillRepeatedly(Return(mockView));
EXPECT_CALL(*pMockViewFactory, getPlotXLabel()).WillRepeatedly(Return("H"));
EXPECT_CALL(*pMockViewFactory, getPlotYLabel()).WillRepeatedly(Return("K"));

Expand All @@ -459,13 +459,13 @@ class ConcretePeaksPresenterTest : public CxxTest::TestSuite
// Create a mock view object/product that will be returned by the mock factory.
auto pMockView = new NiceMock<MockPeakOverlayView>;
auto mockView = boost::shared_ptr<NiceMock<MockPeakOverlayView> >(pMockView);
EXPECT_CALL(*pMockView, showView()).Times(expectedNumberOfPeaks); // Expect that the view will be forced to SHOW.
EXPECT_CALL(*pMockView, hideView()).Times(expectedNumberOfPeaks); // Expect that the view will be forced to HIDE.
EXPECT_CALL(*pMockView, updateView()).Times(2*expectedNumberOfPeaks); // Expect that each widget will be updated.
EXPECT_CALL(*pMockView, showView()).Times(1); // Expect that the view will be forced to SHOW.
EXPECT_CALL(*pMockView, hideView()).Times(1); // Expect that the view will be forced to HIDE.
EXPECT_CALL(*pMockView, updateView()).Times(2); // Expect that each widget will be updated.
// Create a widget factory mock
auto pMockViewFactory = new MockPeakOverlayFactory;
PeakOverlayViewFactory_sptr mockViewFactory = PeakOverlayViewFactory_sptr(pMockViewFactory);
EXPECT_CALL(*pMockViewFactory, createView(_,_)).WillRepeatedly(Return(mockView));
EXPECT_CALL(*pMockViewFactory, createView(_)).WillRepeatedly(Return(mockView));
EXPECT_CALL(*pMockViewFactory, getPlotXLabel()).WillRepeatedly(Return("H"));
EXPECT_CALL(*pMockViewFactory, getPlotYLabel()).WillRepeatedly(Return("K"));

Expand Down Expand Up @@ -505,12 +505,12 @@ class ConcretePeaksPresenterTest : public CxxTest::TestSuite
// Create a mock view object/product that will be returned by the mock factory.
auto pMockView = new NiceMock<MockPeakOverlayView>;
auto mockView = boost::shared_ptr<NiceMock<MockPeakOverlayView> >(pMockView);
EXPECT_CALL(*pMockView, getBoundingBox()).Times(1).WillOnce(Return(PeakBoundingBox())); // Expect that the bounding box will be requested.
EXPECT_CALL(*pMockView, getBoundingBox(_)).Times(1).WillOnce(Return(PeakBoundingBox())); // Expect that the bounding box will be requested.

// Create a widget factory mock
auto pMockViewFactory = new MockPeakOverlayFactory;
PeakOverlayViewFactory_sptr mockViewFactory = PeakOverlayViewFactory_sptr(pMockViewFactory);
EXPECT_CALL(*pMockViewFactory, createView(_,_)).WillRepeatedly(Return(mockView));
EXPECT_CALL(*pMockViewFactory, createView(_)).WillRepeatedly(Return(mockView));
EXPECT_CALL(*pMockViewFactory, getPlotXLabel()).WillRepeatedly(Return("H"));
EXPECT_CALL(*pMockViewFactory, getPlotYLabel()).WillRepeatedly(Return("K"));

Expand All @@ -536,7 +536,7 @@ class ConcretePeaksPresenterTest : public CxxTest::TestSuite
// Create a widget factory mock
auto pMockViewFactory = new MockPeakOverlayFactory;
PeakOverlayViewFactory_sptr mockViewFactory = PeakOverlayViewFactory_sptr(pMockViewFactory);
EXPECT_CALL(*pMockViewFactory, createView(_,_)).WillRepeatedly(Return(mockView));
EXPECT_CALL(*pMockViewFactory, createView(_)).WillRepeatedly(Return(mockView));
EXPECT_CALL(*pMockViewFactory, getPlotXLabel()).WillRepeatedly(Return("H"));
EXPECT_CALL(*pMockViewFactory, getPlotYLabel()).WillRepeatedly(Return("K"));

Expand Down
4 changes: 2 additions & 2 deletions Code/Mantid/MantidQt/SliceViewer/test/MockObjects.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ class MockPeakTransformFactory : public PeakTransformFactory
MOCK_METHOD1(changeForegroundColour, void(const QColor));
MOCK_METHOD1(changeBackgroundColour, void(const QColor));
MOCK_METHOD1(showBackgroundRadius, void(const bool));
MOCK_CONST_METHOD0(getBoundingBox, PeakBoundingBox());
MOCK_CONST_METHOD1(getBoundingBox, PeakBoundingBox(const int));
MOCK_METHOD1(changeOccupancyInView, void(const double));
MOCK_METHOD1(changeOccupancyIntoView, void(const double));
MOCK_CONST_METHOD0(getOccupancyInView, double());
Expand All @@ -127,7 +127,7 @@ class MockPeakTransformFactory : public PeakTransformFactory
class MockPeakOverlayFactory : public PeakOverlayViewFactory
{
public:
MOCK_CONST_METHOD2(createView, boost::shared_ptr<PeakOverlayView>(const int, PeakTransform_const_sptr));
MOCK_CONST_METHOD1(createView, boost::shared_ptr<PeakOverlayView>(PeakTransform_const_sptr));
MOCK_CONST_METHOD0(getPlotXLabel, std::string());
MOCK_CONST_METHOD0(getPlotYLabel, std::string());
MOCK_METHOD0(updateView, void());
Expand Down

0 comments on commit 0874d4d

Please sign in to comment.