Skip to content

Commit

Permalink
Re #12687 Added clone() to Workspaces. Retaining old behavior.
Browse files Browse the repository at this point in the history
Added non-virtual clone() interface with a virtual private doClone()
method to Workspace and derived classes. Workspaces that had a clone()
method before were adapted to have a common interface everywhere. In
cases where there was no clone() method previously we simply throw an
error for now, until this new functionality is implemented.

Some tests/mocks had to be adapted since there there is a pure virtual
doClone() in all abstract classes.

Do to smart pointer incompatibilities (mainly due to old boost versions)
we had do extract the raw pointer from the unique_ptr returned by clone
and put it into a boost::shared_ptr manually, in some cases. See also
issue 12949.
  • Loading branch information
SimonHeybrock committed Jun 30, 2015
1 parent aa45f08 commit 3231d87
Show file tree
Hide file tree
Showing 30 changed files with 192 additions and 51 deletions.
10 changes: 10 additions & 0 deletions Code/Mantid/Framework/API/inc/MantidAPI/IEventWorkspace.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ namespace API {
class MANTID_API_DLL IEventWorkspace : public MatrixWorkspace {
public:
IEventWorkspace() : MatrixWorkspace() {}

/// Returns a clone of the workspace
std::unique_ptr<IEventWorkspace> clone() const {
return std::unique_ptr<IEventWorkspace>(doClone());
}

/// Return the workspace typeID
virtual const std::string id() const { return "IEventWorkspace"; }
virtual std::size_t getNumberEvents() const = 0;
Expand All @@ -64,6 +70,10 @@ class MANTID_API_DLL IEventWorkspace : public MatrixWorkspace {
IEventWorkspace &operator=(const IEventWorkspace &other);

virtual const std::string toString() const;

private:
virtual IEventWorkspace *doClone() const override = 0;

};
}
}
Expand Down
8 changes: 8 additions & 0 deletions Code/Mantid/Framework/API/inc/MantidAPI/IMDEventWorkspace.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ class MANTID_API_DLL IMDEventWorkspace : public API::IMDWorkspace,
IMDEventWorkspace();
virtual ~IMDEventWorkspace() {}

/// Returns a clone of the workspace
std::unique_ptr<IMDEventWorkspace> clone() const {
return std::unique_ptr<IMDEventWorkspace>(doClone());
}

/// Perform initialization after dimensions (and others) have been set.
virtual void initialize() = 0;

Expand Down Expand Up @@ -90,6 +95,9 @@ class MANTID_API_DLL IMDEventWorkspace : public API::IMDWorkspace,
/// Marker set to true when a file-backed workspace needs its back-end file
/// updated (by calling SaveMD(UpdateFileBackEnd=1) )
bool m_fileNeedsUpdating;

private:
virtual IMDEventWorkspace *doClone() const override = 0;
};

} // namespace MDEvents
Expand Down
10 changes: 8 additions & 2 deletions Code/Mantid/Framework/API/inc/MantidAPI/IMDHistoWorkspace.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ class DLLExport IMDHistoWorkspace : public IMDWorkspace,
IMDHistoWorkspace();
virtual ~IMDHistoWorkspace();

/// Returns a clone of the workspace
std::unique_ptr<IMDHistoWorkspace> clone() const {
return std::unique_ptr<IMDHistoWorkspace>(doClone());
}

/// See the MDHistoWorkspace definition for descriptions of these
virtual coord_t getInverseVolume() const = 0;
virtual signal_t *getSignalArray() const = 0;
Expand Down Expand Up @@ -91,8 +96,6 @@ class DLLExport IMDHistoWorkspace : public IMDWorkspace,
virtual void setCoordinateSystem(
const Kernel::SpecialCoordinateSystem coordinateSystem) = 0;

virtual boost::shared_ptr<IMDHistoWorkspace> clone() const = 0;


protected:
/// Protected copy constructor. May be used by childs for cloning.
Expand All @@ -101,6 +104,9 @@ class DLLExport IMDHistoWorkspace : public IMDWorkspace,
IMDHistoWorkspace &operator=(const IMDHistoWorkspace &other);

virtual const std::string toString() const;

private:
virtual IMDHistoWorkspace *doClone() const override = 0;
};

} // namespace API
Expand Down
8 changes: 8 additions & 0 deletions Code/Mantid/Framework/API/inc/MantidAPI/IMDWorkspace.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ class MANTID_API_DLL IMDWorkspace : public Workspace, public API::MDGeometry {
IMDWorkspace();
virtual ~IMDWorkspace();

/// Returns a clone of the workspace
std::unique_ptr<IMDWorkspace> clone() const {
return std::unique_ptr<IMDWorkspace>(doClone());
}

/// Get the number of points associated with the workspace.
/// For MDEvenWorkspace it is the number of events contributing into the
/// workspace
Expand Down Expand Up @@ -139,6 +144,9 @@ class MANTID_API_DLL IMDWorkspace : public Workspace, public API::MDGeometry {
IMDWorkspace &operator=(const IMDWorkspace &other);

virtual const std::string toString() const;

private:
virtual IMDWorkspace *doClone() const override = 0;
};

/// Shared pointer to the IMDWorkspace base class
Expand Down
8 changes: 8 additions & 0 deletions Code/Mantid/Framework/API/inc/MantidAPI/IPeaksWorkspace.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ class MANTID_API_DLL IPeaksWorkspace : public ITableWorkspace,
/// Destructor
virtual ~IPeaksWorkspace();

/// Returns a clone of the workspace
std::unique_ptr<IPeaksWorkspace> clone() const {
return std::unique_ptr<IPeaksWorkspace>(doClone());
}

//---------------------------------------------------------------------------------------------
/** @return the number of peaks
*/
Expand Down Expand Up @@ -154,6 +159,9 @@ class MANTID_API_DLL IPeaksWorkspace : public ITableWorkspace,
IPeaksWorkspace &operator=(const IPeaksWorkspace &other);

virtual const std::string toString() const;

private:
virtual IPeaksWorkspace *doClone() const override = 0;
};

}
Expand Down
11 changes: 8 additions & 3 deletions Code/Mantid/Framework/API/inc/MantidAPI/ITableWorkspace.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,11 @@ class MANTID_API_DLL ITableWorkspace : public API::Workspace {
/// Virtual destructor.
virtual ~ITableWorkspace() {}

/// Returns a clone of the workspace
std::unique_ptr<ITableWorkspace> clone() const {
return std::unique_ptr<ITableWorkspace>(doClone());
}

/// Return the workspace typeID
virtual const std::string id() const { return "ITableWorkspace"; }
virtual const std::string toString() const;
Expand All @@ -144,9 +149,6 @@ class MANTID_API_DLL ITableWorkspace : public API::Workspace {
/// Removes a column.
virtual void removeColumn(const std::string &name) = 0;

/// Clones the table workspace
virtual ITableWorkspace *clone() const = 0;

/// Number of columns in the workspace.
virtual size_t columnCount() const = 0;

Expand Down Expand Up @@ -329,6 +331,9 @@ class MANTID_API_DLL ITableWorkspace : public API::Workspace {
@param index :: Index of the element to be removed.
*/
void removeFromColumn(Column *c, size_t index) { c->remove(index); }

private:
virtual ITableWorkspace *doClone() const override = 0;
};

// =====================================================================================
Expand Down
7 changes: 7 additions & 0 deletions Code/Mantid/Framework/API/inc/MantidAPI/MatrixWorkspace.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ class MANTID_API_DLL MatrixWorkspace : public IMDWorkspace,
/// Delete
virtual ~MatrixWorkspace();

/// Returns a clone of the workspace
std::unique_ptr<MatrixWorkspace> clone() const {
return std::unique_ptr<MatrixWorkspace>(doClone());
}

using IMDWorkspace::toString;
/// String description of state
const std::string toString() const;
Expand Down Expand Up @@ -446,6 +451,8 @@ class MANTID_API_DLL MatrixWorkspace : public IMDWorkspace,
std::vector<Axis *> m_axes;

private:
virtual MatrixWorkspace *doClone() const override = 0;

/// Create an MantidImage instance.
MantidImage_sptr
getImage(const MantidVec &(MatrixWorkspace::*read)(std::size_t const) const,
Expand Down
26 changes: 26 additions & 0 deletions Code/Mantid/Framework/API/inc/MantidAPI/Workspace.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,29 @@ class MANTID_API_DLL Workspace : public Kernel::DataItem {
Workspace();
virtual ~Workspace();

/** Returns a clone (copy) of the workspace with covariant return type in all
* derived classes.
*
* Note that this public function is *not* virtual. This has two reasons:
* - We want to enforce covariant return types. If this public clone() method
* was virtual some derived class might fail to reimplement it. Since there
* are several levels of inheritance in the Workspace inheritance tree we
* cannot just use a pure virtual method in the base class. Thus, we use
* this non-virtual interface method which calls the private and virtual
* doClone() method. Since it is private, failing to reimplement it will
* cause a compiler error as a reminder. Note that this mechanism does not
* always work if a derived class does not implement clone(): if doClone()
* in a parent is implemented then calling clone on a base class will return
* a clone of the parent defining doClone, not the actual instance. This is
* more a problem of the inheritance structure, i.e., whether or not all
* non-leaf classes are pure virtual and declare doClone()=0.
* - Covariant return types are in conflict with smart pointers, but if
* clone() is not virtual this is a non-issue.
*/
std::unique_ptr<Workspace> clone() const {
return std::unique_ptr<Workspace>(doClone());
}

// DataItem interface
/// Name
virtual const std::string name() const { return this->getName(); }
Expand Down Expand Up @@ -101,6 +124,9 @@ class MANTID_API_DLL Workspace : public Kernel::DataItem {
/// The history of the workspace, algorithm and environment
WorkspaceHistory m_history;

/// Virtual clone method. Not implemented to force implementation in childs.
virtual Workspace *doClone() const = 0;

friend class AnalysisDataServiceImpl;
};

Expand Down
3 changes: 3 additions & 0 deletions Code/Mantid/Framework/API/inc/MantidAPI/WorkspaceGroup.h
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,9 @@ class MANTID_API_DLL WorkspaceGroup : public Workspace {
const WorkspaceGroup &operator=(const WorkspaceGroup &);

private:
virtual WorkspaceGroup *doClone() const override {
throw std::runtime_error("Cloning of WorkspaceGroup is not implemented.");
}
/// ADS removes a member of this group using this method. It doesn't send
/// notifications in contrast to remove(name).
void removeByADS(const std::string &name);
Expand Down
5 changes: 5 additions & 0 deletions Code/Mantid/Framework/API/test/AnalysisDataServiceTest.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ namespace
virtual const std::string id() const { return "MockWorkspace"; }
virtual const std::string toString() const { return ""; }
virtual size_t getMemorySize() const { return 1; }

private:
virtual MockWorkspace *doClone() const override {
throw std::runtime_error("Cloning of MockWorkspace is not implemented.");
}
};
typedef boost::shared_ptr<MockWorkspace> MockWorkspace_sptr;
}
Expand Down
5 changes: 5 additions & 0 deletions Code/Mantid/Framework/API/test/CompositeFunctionTest.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@ class CompositeFunctionTest_MocMatrixWorkspace : public MatrixWorkspace
void clearFileBacked(bool ){};
ITableWorkspace_sptr makeBoxTable(size_t /* start*/, size_t /*num*/){return ITableWorkspace_sptr();}
private:
virtual CompositeFunctionTest_MocMatrixWorkspace *doClone() const override {
throw std::runtime_error("Cloning of "
"CompositeFunctionTest_MocMatrixWorkspace is not "
"implemented.");
}
std::vector<CompositeFunctionTest_MocSpectrum> m_spectra;
size_t m_blocksize;
};
Expand Down
5 changes: 5 additions & 0 deletions Code/Mantid/Framework/API/test/ScopedWorkspaceTest.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ class MockWorkspace : public Workspace
virtual const std::string id() const { return "MockWorkspace"; }
virtual const std::string toString() const { return ""; }
virtual size_t getMemorySize() const { return 1; }

private:
virtual MockWorkspace *doClone() const override {
throw std::runtime_error("Cloning of MockWorkspace is not implemented.");
}
};
typedef boost::shared_ptr<MockWorkspace> MockWorkspace_sptr;

Expand Down
5 changes: 5 additions & 0 deletions Code/Mantid/Framework/API/test/WorkspaceGroupTest.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ class WorkspaceGroupTest : public CxxTest::TestSuite
MOCK_CONST_METHOD0(threadSafe, bool());
MOCK_CONST_METHOD0(toString, const std::string());
MOCK_CONST_METHOD0(getMemorySize, size_t());

private:
virtual MockWorkspace *doClone() const override {
throw std::runtime_error("Cloning of MockWorkspace is not implemented.");
}
};

/// Make a simple group
Expand Down
6 changes: 6 additions & 0 deletions Code/Mantid/Framework/Algorithms/test/RebinByTimeBaseTest.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,12 @@ namespace
MOCK_CONST_METHOD0(getSpecialCoordinateSystem, Mantid::Kernel::SpecialCoordinateSystem());
virtual ~MockIEventWorkspace()
{}

private:
virtual MockIEventWorkspace *doClone() const override {
throw std::runtime_error(
"Cloning of MockIEventWorkspace is not implemented.");
}
};}
//=====================================================================================
// Functional Tests
Expand Down
3 changes: 1 addition & 2 deletions Code/Mantid/Framework/Crystal/src/IntegratePeaksHybrid.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,7 @@ void IntegratePeaksHybrid::exec() {
const double peakOuterRadius = getProperty("BackgroundOuterRadius");
const double halfPeakOuterRadius = peakOuterRadius / 2;
if (peakWS != inPeakWS) {
peakWS = IPeaksWorkspace_sptr(
dynamic_cast<IPeaksWorkspace *>(inPeakWS->clone()));
peakWS = IPeaksWorkspace_sptr(inPeakWS->clone().release());
}

{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,7 @@ void IntegratePeaksUsingClusters::exec() {
IPeaksWorkspace_sptr inPeakWS = getProperty("PeaksWorkspace");
IPeaksWorkspace_sptr peakWS = getProperty("OutputWorkspace");
if (peakWS != inPeakWS) {
peakWS = IPeaksWorkspace_sptr(
dynamic_cast<IPeaksWorkspace *>(inPeakWS->clone()));
peakWS = IPeaksWorkspace_sptr(inPeakWS->clone().release());
}

{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ class DLLExport EventWorkspace : public API::IEventWorkspace {
// Destructor
virtual ~EventWorkspace();

/// Returns a clone of the workspace
std::unique_ptr<EventWorkspace> clone() const {
return std::unique_ptr<EventWorkspace>(doClone());
}

// Initialize the pixels
void init(const std::size_t &, const std::size_t &, const std::size_t &);

Expand Down Expand Up @@ -196,6 +201,10 @@ class DLLExport EventWorkspace : public API::IEventWorkspace {
EventWorkspace &operator=(const EventWorkspace &other);

private:
virtual EventWorkspace *doClone() const override {
throw std::runtime_error("Cloning of EventWorkspace is not implemented.");
}

/** A vector that holds the event list for each spectrum; the key is
* the workspace index, which is not necessarily the pixelid.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ class DLLExport MDEventWorkspace : public API::IMDEventWorkspace {
MDEventWorkspace(const MDEventWorkspace<MDE, nd> &other);
virtual ~MDEventWorkspace();

/// Returns a clone of the workspace
std::unique_ptr<MDEventWorkspace> clone() const {
return std::unique_ptr<MDEventWorkspace>(doClone());
}

/// Perform initialization after dimensions (and others) have been set.
virtual void initialize();

Expand Down Expand Up @@ -185,6 +190,10 @@ class DLLExport MDEventWorkspace : public API::IMDEventWorkspace {
API::BoxController_sptr m_BoxController;
// boost::shared_ptr<BoxCtrlChangesList > m_BoxController;
private:
virtual MDEventWorkspace *doClone() const override {
throw std::runtime_error("Cloning of MDEventWorkspace is not implemented.");
}

Kernel::SpecialCoordinateSystem m_coordSystem;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@ class DLLExport MDHistoWorkspace : public API::IMDHistoWorkspace {

virtual ~MDHistoWorkspace();

/// Returns a clone of the workspace
std::unique_ptr<MDHistoWorkspace> clone() const {
return std::unique_ptr<MDHistoWorkspace>(doClone());
}

void init(std::vector<Mantid::Geometry::MDHistoDimension_sptr> &dimensions);
void init(std::vector<Mantid::Geometry::IMDDimension_sptr> &dimensions);

Expand Down Expand Up @@ -379,13 +384,14 @@ class DLLExport MDHistoWorkspace : public API::IMDHistoWorkspace {
/// Get the size of an element in the HistoWorkspace.
static size_t sizeOfElement();

/// Virutal constructor.
boost::shared_ptr<IMDHistoWorkspace> clone() const;

/// Preferred visual normalization method.
virtual Mantid::API::MDNormalization displayNormalization() const;

private:
virtual MDHistoWorkspace *doClone() const override {
return new MDHistoWorkspace(*this);
}

void initVertexesArray();

/// Number of dimensions in this workspace
Expand Down

0 comments on commit 3231d87

Please sign in to comment.