Skip to content

Commit abe848a

Browse files
adriazalvarezrboston628
authored andcommitted
Remove nested boost::optionals in ISISReflectometry (#39223)
* Add validatorT typedef as a std::pair of boost::optional<T> and bool to avoid the need for nested boost:optional * Refactor variadic template functions to validate all table fields are properly initialized, and use simpler fold syntax to check validity of all rows * Change boost optional in common classes * - Changed boost to std optional in batch classes - Change boost::optional<T&> to std::optional<std::reference_wrapper<T>> * - Changed boost to std optional in decoder/encoder * - Changed boost to std optional in experiment and preview classes * - Changed boost to std optional in Runs and RunsTable * - Changed boost to std optional in Reduction * - Make tests compatible with changes to std::optional in IsisReflectometry * - Change more boost optional in ISIS Reflectometry * - Remove unused allinitialized file * Fix a bunch of cppcheck suppresions * Move TaggetOptional out of validation result * remove unnecesary include * Make sure boost::optional contains values before initializing clipboard * Move TaggedOptional into ParseReflectometryStrings * Fix broken scale factor test * Remove unnecesary creation of optionals for the row objects and rename variables to more appropriate names as nested boost optional is no longer used * Add extra check for std::nullopt in parse scale factor test
1 parent c462aaa commit abe848a

File tree

97 files changed

+1048
-1088
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

97 files changed

+1048
-1088
lines changed

buildconfig/CMake/CppCheck_Suppressions.txt.in

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -488,7 +488,6 @@ constVariablePointer:${CMAKE_SOURCE_DIR}/Framework/WorkflowAlgorithms/src/HFIRLo
488488
unreadVariable:${CMAKE_SOURCE_DIR}/Framework/WorkflowAlgorithms/src/SofTwoThetaTOF.cpp:196
489489
missingOverride:${CMAKE_SOURCE_DIR}/Testing/Tools/cxxtest/cxxtest/GlobalFixture.h:22
490490
constParameterReference:${CMAKE_SOURCE_DIR}/qt/icons/src/Icon.cpp:80
491-
constVariableReference:${CMAKE_SOURCE_DIR}/qt/scientific_interfaces/ISISReflectometry/Common/Clipboard.cpp:80
492491
passedByValue:${CMAKE_SOURCE_DIR}/qt/scientific_interfaces/ISISReflectometry/Common/OptionDefaults.h:47
493492
missingOverride:${CMAKE_SOURCE_DIR}/qt/scientific_interfaces/ISISReflectometry/GUI/Common/IReflMessageHandler.h:21
494493
constParameterReference:${CMAKE_SOURCE_DIR}/qt/scientific_interfaces/ISISReflectometry/GUI/Experiment/QtExperimentView.cpp:267
@@ -505,16 +504,9 @@ constParameterReference:${CMAKE_SOURCE_DIR}/qt/scientific_interfaces/ISISReflect
505504
constParameterReference:${CMAKE_SOURCE_DIR}/qt/scientific_interfaces/ISISReflectometry/GUI/Instrument/QtInstrumentView.cpp:91
506505
constParameterReference:${CMAKE_SOURCE_DIR}/qt/scientific_interfaces/ISISReflectometry/GUI/Instrument/QtInstrumentView.cpp:95
507506
missingOverride:${CMAKE_SOURCE_DIR}/qt/scientific_interfaces/ISISReflectometry/GUI/Options/OptionsDialogModel.h:20
508-
missingOverride:${CMAKE_SOURCE_DIR}/qt/scientific_interfaces/ISISReflectometry/GUI/Preview/PreviewModel.h:31
509507
missingOverride:${CMAKE_SOURCE_DIR}/qt/scientific_interfaces/ISISReflectometry/GUI/Runs/IRunsView.h:66
510508
missingOverride:${CMAKE_SOURCE_DIR}/qt/scientific_interfaces/ISISReflectometry/GUI/Runs/QtCatalogSearcher.h:32
511509
missingOverride:${CMAKE_SOURCE_DIR}/qt/scientific_interfaces/ISISReflectometry/GUI/RunsTable/IRunsTableView.h:31
512-
constVariablePointer:${CMAKE_SOURCE_DIR}/qt/scientific_interfaces/ISISReflectometry/GUI/RunsTable/RunsTablePresenter.cpp:510
513-
constVariableReference:${CMAKE_SOURCE_DIR}/qt/scientific_interfaces/ISISReflectometry/GUI/RunsTable/RunsTablePresenter.cpp:749
514-
constVariableReference:${CMAKE_SOURCE_DIR}/qt/scientific_interfaces/ISISReflectometry/GUI/RunsTable/RunsTablePresenter.cpp:754
515-
constVariableReference:${CMAKE_SOURCE_DIR}/qt/scientific_interfaces/ISISReflectometry/GUI/RunsTable/RunsTablePresenter.cpp:785
516-
unreadVariable:${CMAKE_SOURCE_DIR}/qt/scientific_interfaces/ISISReflectometry/GUI/RunsTable/RunsTablePresenter.cpp:786
517-
constVariableReference:${CMAKE_SOURCE_DIR}/qt/scientific_interfaces/ISISReflectometry/GUI/RunsTable/RunsTablePresenter.cpp:788
518510
virtualCallInConstructor:${CMAKE_SOURCE_DIR}/qt/scientific_interfaces/ISISReflectometry/GUI/RunsTable/RunsTablePresenter.h:60
519511
missingOverride:${CMAKE_SOURCE_DIR}/qt/scientific_interfaces/ISISReflectometry/GUI/Save/SaveAlgorithmRunner.h:18
520512
returnByReference:${CMAKE_SOURCE_DIR}/qt/scientific_interfaces/ISISReflectometry/Reduction/Experiment.h:54

qt/scientific_interfaces/ISISReflectometry/Common/Clipboard.cpp

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,13 @@
1313

1414
namespace MantidQt::CustomInterfaces::ISISReflectometry {
1515

16-
Clipboard::Clipboard() : m_subtrees(boost::none), m_subtreeRoots(boost::none) {}
16+
Clipboard::Clipboard() : m_subtrees(std::nullopt), m_subtreeRoots(std::nullopt) {}
1717

18-
Clipboard::Clipboard(boost::optional<std::vector<MantidQt::MantidWidgets::Batch::Subtree>> subtrees,
19-
boost::optional<std::vector<MantidQt::MantidWidgets::Batch::RowLocation>> subtreeRoots)
20-
: m_subtrees(std::move(subtrees)), m_subtreeRoots(std::move(subtreeRoots)) {}
18+
Clipboard::Clipboard(std::vector<MantidQt::MantidWidgets::Batch::Subtree> subtrees,
19+
std::vector<MantidQt::MantidWidgets::Batch::RowLocation> subtreeRoots)
20+
: m_subtrees(std::optional(subtrees)), m_subtreeRoots(std::optional(subtreeRoots)) {}
2121

22-
bool Clipboard::isInitialized() const { return m_subtrees.is_initialized() && m_subtreeRoots.is_initialized(); }
22+
bool Clipboard::isInitialized() const { return m_subtrees.has_value() && m_subtreeRoots.has_value(); }
2323

2424
int Clipboard::numberOfRoots() const {
2525
if (!isInitialized())
@@ -77,30 +77,30 @@ Group Clipboard::createGroupForRoot(int rootIndex) const {
7777

7878
auto result = Group(groupName(rootIndex));
7979
auto rowsToAdd = createRowsForRootChildren(rootIndex);
80-
for (auto &row : rowsToAdd)
80+
for (auto const &row : rowsToAdd)
8181
result.appendRow(row);
8282
return result;
8383
}
8484

85-
std::vector<boost::optional<Row>> Clipboard::createRowsForAllRoots() const {
85+
std::vector<std::optional<Row>> Clipboard::createRowsForAllRoots() const {
8686
if (containsGroups(*this))
8787
throw std::runtime_error("Attempted to get row for group clipboard item");
8888

89-
auto result = std::vector<boost::optional<Row>>();
89+
auto result = std::vector<std::optional<Row>>();
9090
std::for_each(subtrees().cbegin(), subtrees().cend(), [this, &result](const auto &subtree) {
9191
const auto rowsToAdd = createRowsForSubtree(subtree);
9292
std::copy(rowsToAdd.cbegin(), rowsToAdd.cend(), std::back_inserter(result));
9393
});
9494
return result;
9595
}
9696

97-
std::vector<boost::optional<Row>> Clipboard::createRowsForRootChildren(int rootIndex) const {
97+
std::vector<std::optional<Row>> Clipboard::createRowsForRootChildren(int rootIndex) const {
9898
return createRowsForSubtree(subtrees()[rootIndex]);
9999
}
100100

101-
std::vector<boost::optional<Row>>
101+
std::vector<std::optional<Row>>
102102
Clipboard::createRowsForSubtree(MantidQt::MantidWidgets::Batch::Subtree const &subtree) const {
103-
auto result = std::vector<boost::optional<Row>>();
103+
auto result = std::vector<std::optional<Row>>();
104104

105105
for (auto const &row : subtree) {
106106
// Skip the root item if it is a group
@@ -111,25 +111,26 @@ Clipboard::createRowsForSubtree(MantidQt::MantidWidgets::Batch::Subtree const &s
111111
std::transform(row.cells().cbegin(), row.cells().cend(), std::back_inserter(cells),
112112
[](MantidQt::MantidWidgets::Batch::Cell const &cell) { return cell.contentText(); });
113113
auto validationResult = validateRow(cells);
114-
if (validationResult.isValid())
114+
if (validationResult.isValid()) {
115115
result.emplace_back(validationResult.assertValid());
116-
else
117-
result.emplace_back(boost::none);
116+
} else {
117+
result.emplace_back(std::nullopt);
118+
}
118119
}
119120

120121
return result;
121122
}
122123

123-
std::vector<MantidQt::MantidWidgets::Batch::Subtree> &Clipboard::mutableSubtrees() { return m_subtrees.get(); }
124+
std::vector<MantidQt::MantidWidgets::Batch::Subtree> &Clipboard::mutableSubtrees() { return m_subtrees.value(); }
124125

125-
std::vector<MantidQt::MantidWidgets::Batch::Subtree> const &Clipboard::subtrees() const { return m_subtrees.get(); }
126+
std::vector<MantidQt::MantidWidgets::Batch::Subtree> const &Clipboard::subtrees() const { return m_subtrees.value(); }
126127

127128
std::vector<MantidQt::MantidWidgets::Batch::RowLocation> const &Clipboard::subtreeRoots() const {
128-
return m_subtreeRoots.get();
129+
return m_subtreeRoots.value();
129130
}
130131

131132
std::vector<MantidQt::MantidWidgets::Batch::RowLocation> &Clipboard::mutableSubtreeRoots() {
132-
return m_subtreeRoots.get();
133+
return m_subtreeRoots.value();
133134
}
134135

135136
bool containsGroups(Clipboard const &clipboard) {

qt/scientific_interfaces/ISISReflectometry/Common/Clipboard.h

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,16 +25,16 @@ class MANTIDQT_ISISREFLECTOMETRY_DLL Clipboard {
2525
};
2626

2727
Clipboard();
28-
Clipboard(boost::optional<std::vector<MantidQt::MantidWidgets::Batch::Subtree>> subtrees,
29-
boost::optional<std::vector<MantidQt::MantidWidgets::Batch::RowLocation>> subtreeRoots);
28+
Clipboard(std::vector<MantidQt::MantidWidgets::Batch::Subtree> subtrees,
29+
std::vector<MantidQt::MantidWidgets::Batch::RowLocation> subtreeRoots);
3030

3131
bool isInitialized() const;
3232
int numberOfRoots() const;
3333
bool isGroupLocation(int rootIndex) const;
3434
std::string groupName(int rootIndex) const;
3535
void setGroupName(int rootIndex, std::string const &groupName);
3636
Group createGroupForRoot(int rootIndex) const;
37-
std::vector<boost::optional<Row>> createRowsForAllRoots() const;
37+
std::vector<std::optional<Row>> createRowsForAllRoots() const;
3838

3939
std::vector<MantidQt::MantidWidgets::Batch::Subtree> const &subtrees() const;
4040
std::vector<MantidQt::MantidWidgets::Batch::Subtree> &mutableSubtrees();
@@ -44,16 +44,16 @@ class MANTIDQT_ISISREFLECTOMETRY_DLL Clipboard {
4444
private:
4545
// The subtrees for each of the roots. Note that the Rows here contain
4646
// relative paths
47-
boost::optional<std::vector<MantidQt::MantidWidgets::Batch::Subtree>> m_subtrees;
47+
std::optional<std::vector<MantidQt::MantidWidgets::Batch::Subtree>> m_subtrees;
4848
// The actual locations of the roots that were copied. This allows us to work
4949
// out the actual paths that were copied and determine whether items are rows
5050
// or groups in the reflectometry GUI sense. Note that these locations may
5151
// not be valid in the table if other edits have been made so this should
5252
// only be used for checking whether copied values were rows/groups.
53-
boost::optional<std::vector<MantidQt::MantidWidgets::Batch::RowLocation>> m_subtreeRoots;
53+
std::optional<std::vector<MantidQt::MantidWidgets::Batch::RowLocation>> m_subtreeRoots;
5454

55-
std::vector<boost::optional<Row>> createRowsForRootChildren(int rootIndex) const;
56-
std::vector<boost::optional<Row>> createRowsForSubtree(MantidQt::MantidWidgets::Batch::Subtree const &subtree) const;
55+
std::vector<std::optional<Row>> createRowsForRootChildren(int rootIndex) const;
56+
std::vector<std::optional<Row>> createRowsForSubtree(MantidQt::MantidWidgets::Batch::Subtree const &subtree) const;
5757
};
5858

5959
bool MANTIDQT_ISISREFLECTOMETRY_DLL containsGroups(Clipboard const &clipboard);

qt/scientific_interfaces/ISISReflectometry/Common/First.h

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,34 +5,34 @@
55
// Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS
66
// SPDX - License - Identifier: GPL - 3.0 +
77
#pragma once
8-
#include <boost/optional.hpp>
98
#include <boost/variant.hpp>
9+
#include <optional>
1010
#include <vector>
1111

1212
namespace MantidQt {
1313
namespace CustomInterfaces {
1414
namespace ISISReflectometry {
1515

16-
template <typename T> boost::optional<T> first(std::vector<T> const &values) {
17-
if (values.size() > 0)
18-
return boost::optional<T>(values[0]);
19-
else
20-
return boost::none;
16+
template <typename T> std::optional<T> first(std::vector<T> const &values) {
17+
if (values.size() > 0) {
18+
return std::optional<T>(values[0]);
19+
}
20+
return std::nullopt;
2121
}
2222

2323
/**
2424
* Operates on a variant<vector<Ts>...> extracting the first element and
2525
* returning it as a optional<variant<T>> where the optional is empty if
2626
* the vector held by the variant<vector<T>> held no values.
2727
*/
28-
template <typename... Ts> class FirstVisitor : public boost::static_visitor<boost::optional<boost::variant<Ts...>>> {
28+
template <typename... Ts> class FirstVisitor : public boost::static_visitor<std::optional<boost::variant<Ts...>>> {
2929
public:
30-
template <typename T> boost::optional<boost::variant<Ts...>> operator()(std::vector<T> const &values) const {
30+
template <typename T> std::optional<boost::variant<Ts...>> operator()(std::vector<T> const &values) const {
3131
auto value = first(values);
32-
if (value)
32+
if (value) {
3333
return boost::variant<Ts...>(value.get());
34-
else
35-
return boost::none;
34+
}
35+
return std::nullopt;
3636
}
3737
};
3838
} // namespace ISISReflectometry

qt/scientific_interfaces/ISISReflectometry/Common/InstrumentParameters.h

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,22 +8,22 @@
88
#include "Common/First.h"
99
#include "Common/ValueOr.h"
1010
#include "GetInstrumentParameter.h"
11-
#include <boost/optional.hpp>
11+
#include <optional>
1212
#include <vector>
1313

1414
namespace MantidQt {
1515
namespace CustomInterfaces {
1616
namespace ISISReflectometry {
1717

1818
template <typename T>
19-
boost::optional<T> firstFromParameterFile(Mantid::Geometry::Instrument_const_sptr instrument,
20-
std::string const &parameterName) {
19+
std::optional<T> firstFromParameterFile(Mantid::Geometry::Instrument_const_sptr instrument,
20+
std::string const &parameterName) {
2121
return first(getInstrumentParameter<T>(instrument, parameterName));
2222
}
2323

2424
template <typename... Ts>
25-
boost::optional<boost::variant<Ts...>> firstFromParameterFileVariant(Mantid::Geometry::Instrument_const_sptr instrument,
26-
std::string const &parameterName) {
25+
std::optional<boost::variant<Ts...>> firstFromParameterFileVariant(Mantid::Geometry::Instrument_const_sptr instrument,
26+
std::string const &parameterName) {
2727
auto values = getInstrumentParameter<boost::variant<Ts...>>(instrument, parameterName);
2828
return boost::apply_visitor(FirstVisitor<Ts...>(), values);
2929
}
@@ -52,18 +52,17 @@ class InstrumentParameters {
5252
return fromFileOrDefaultConstruct<T>(parameterName);
5353
}
5454

55-
template <typename T> boost::optional<T> optional(std::string const &parameterName) {
55+
template <typename T> std::optional<T> optional(std::string const &parameterName) {
5656
return fromFile<T>(parameterName);
5757
}
5858

5959
template <typename Default, typename T>
60-
T handleMandatoryIfMissing(boost::optional<T> const &value, std::string const &parameterName) {
61-
if (value)
62-
return value.get();
63-
else {
64-
m_missingValueErrors.emplace_back(parameterName);
65-
return Default();
60+
T handleMandatoryIfMissing(std::optional<T> const &value, std::string const &parameterName) {
61+
if (value) {
62+
return value.value();
6663
}
64+
m_missingValueErrors.emplace_back(parameterName);
65+
return Default();
6766
}
6867

6968
template <typename T> T mandatory(std::string const &parameterName) {
@@ -107,12 +106,12 @@ class InstrumentParameters {
107106
return value_or(fromFile<T>(parameterName), T());
108107
}
109108

110-
template <typename T> boost::optional<T> fromFile(std::string const &parameterName) {
109+
template <typename T> std::optional<T> fromFile(std::string const &parameterName) {
111110
try {
112111
return firstFromParameterFile<T>(m_instrument, parameterName);
113112
} catch (InstrumentParameterTypeMissmatch const &ex) {
114113
m_typeErrors.emplace_back(ex);
115-
return boost::none;
114+
return std::nullopt;
116115
}
117116
}
118117

qt/scientific_interfaces/ISISReflectometry/Common/ValidationResult.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
// Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS
66
// SPDX - License - Identifier: GPL - 3.0 +
77
#pragma once
8-
#include <boost/optional.hpp>
98
#include <boost/variant.hpp>
9+
#include <optional>
1010

1111
namespace MantidQt {
1212
namespace CustomInterfaces {
@@ -22,7 +22,7 @@ template <typename Validated, typename Error = boost::blank> class ValidationRes
2222
bool isError() const;
2323
Validated const &assertValid() const;
2424
Error const &assertError() const;
25-
boost::optional<Validated> validElseNone() const;
25+
std::optional<Validated> validElseNone() const;
2626

2727
private:
2828
boost::variant<Validated, Error> m_innerResult;
@@ -55,11 +55,11 @@ template <typename Validated, typename Error> Error const &ValidationResult<Vali
5555
}
5656

5757
template <typename Validated, typename Error>
58-
boost::optional<Validated> ValidationResult<Validated, Error>::validElseNone() const {
59-
if (isValid())
58+
std::optional<Validated> ValidationResult<Validated, Error>::validElseNone() const {
59+
if (isValid()) {
6060
return assertValid();
61-
else
62-
return boost::none;
61+
}
62+
return std::nullopt;
6363
}
6464
} // namespace ISISReflectometry
6565
} // namespace CustomInterfaces

0 commit comments

Comments
 (0)