Skip to content

Commit 6c35e5a

Browse files
committed
Refactor custom group validation function in model to handle better exceptions throws and wrong input formats
1 parent 89732fe commit 6c35e5a

File tree

2 files changed

+31
-34
lines changed

2 files changed

+31
-34
lines changed

qt/scientific_interfaces/Muon/ALCDataLoadingModel.cpp

Lines changed: 31 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,24 @@
2020

2121
using namespace Mantid::API;
2222

23+
namespace ALCGroupingHelper {
24+
/**
25+
* Check basic group string is valid
26+
* i.e. does not contain letters or start or ends with , or -
27+
* @param group :: the string of the grouping
28+
* @returns :: True if group format is incorrect, false otherwise
29+
*/
30+
bool isCustomGroupingInvalid(const std::string &group) {
31+
32+
auto isDecimal = [](const char c) { return c == '.'; };
33+
34+
return !std::isdigit(static_cast<unsigned char>(group.front())) ||
35+
!std::isdigit(static_cast<unsigned char>(group.back())) ||
36+
std::any_of(std::begin(group), std::end(group), ::isalpha) ||
37+
std::any_of(std::begin(group), std::end(group), isDecimal);
38+
}
39+
} // namespace ALCGroupingHelper
40+
2341
namespace MantidQt::CustomInterfaces {
2442

2543
ALCDataLoadingModel::ALCDataLoadingModel()
@@ -249,42 +267,22 @@ bool ALCDataLoadingModel::checkCustomGrouping(const std::string &detGroupingType
249267
if (detGroupingType != "Custom") {
250268
return true;
251269
}
270+
try {
271+
const std::vector<std::string> groupings = {forwardGrouping, backwardGrouping};
272+
std::vector<int> allDetectors;
273+
for (const auto &customGroup : groupings) {
274+
if (ALCGroupingHelper::isCustomGroupingInvalid(customGroup)) {
275+
return false;
276+
}
277+
const auto groupDetectors = Mantid::Kernel::Strings::parseRange(customGroup);
278+
allDetectors.insert(allDetectors.end(), groupDetectors.cbegin(), groupDetectors.cend());
279+
}
252280

253-
bool groupingOK = true;
254-
255-
auto detectors = Mantid::Kernel::Strings::parseRange(isCustomGroupingValid(forwardGrouping, groupingOK));
256-
if (!groupingOK) {
281+
auto isOutsideDetRange = [this](const auto det) { return det < 0 || det > static_cast<int>(m_numDetectors); };
282+
return !std::any_of(allDetectors.cbegin(), allDetectors.cend(), isOutsideDetRange);
283+
} catch (std::invalid_argument &) {
257284
return false;
258285
}
259-
const auto backward = Mantid::Kernel::Strings::parseRange(isCustomGroupingValid(backwardGrouping, groupingOK));
260-
if (!groupingOK) {
261-
return false;
262-
}
263-
264-
detectors.insert(detectors.end(), backward.begin(), backward.end());
265-
auto isOutsideDetRange = [this](const auto det) { return det < 0 || det > static_cast<int>(m_numDetectors); };
266-
267-
return !std::any_of(detectors.cbegin(), detectors.cend(), isOutsideDetRange);
268-
}
269-
270-
/**
271-
* Check basic group string is valid
272-
* i.e. does not contain letters or start with , or -
273-
* @param group :: the string of the grouping
274-
* @param isValid :: bool to say if the string is valid
275-
* @returns :: True if grouping OK, false if bad
276-
*/
277-
std::string ALCDataLoadingModel::isCustomGroupingValid(const std::string &group, bool &isValid) {
278-
279-
auto isDecimal = [](const char c) { return c == '.'; };
280-
281-
if (!std::isdigit(static_cast<unsigned char>(group[0])) ||
282-
std::any_of(std::begin(group), std::end(group), ::isalpha) ||
283-
std::any_of(std::begin(group), std::end(group), isDecimal)) {
284-
isValid = false;
285-
return "";
286-
}
287-
return group;
288286
}
289287

290288
void ALCDataLoadingModel::updateAutoLoadCancelled() {

qt/scientific_interfaces/Muon/ALCDataLoadingModel.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ class MANTIDQT_MUONINTERFACE_DLL ALCDataLoadingModel : public IALCDataLoadingMod
5454
void setFilesToLoad(const std::vector<std::string> &files) override;
5555

5656
private:
57-
static std::string isCustomGroupingValid(const std::string &group, bool &isValid);
5857
static int extractRunNumber(const std::string &file);
5958

6059
/// Last loaded data workspace

0 commit comments

Comments
 (0)