Skip to content

Commit

Permalink
Merge pull request mantidproject#36284 from mantidproject/patch-enume…
Browse files Browse the repository at this point in the history
…rated-string2

type security to enumerated string, extra test of non-class enum
  • Loading branch information
AndreiSavici authored Oct 16, 2023
2 parents 0925e95 + 3ebc5d5 commit c47df2f
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 5 deletions.
4 changes: 4 additions & 0 deletions Framework/Kernel/inc/MantidKernel/EnumeratedString.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <boost/algorithm/string.hpp>
#include <sstream>
#include <string>
#include <type_traits>
#include <vector>

namespace Mantid {
Expand Down Expand Up @@ -38,6 +39,9 @@ class EnumeratedString {
*
* @tparam an optional pointer to a statically defined string comparator.
*/

static_assert(std::is_enum_v<E>); // cause a compiler error if E is not an enum

public:
EnumeratedString() { ensureCompatibleSize(); }

Expand Down
31 changes: 28 additions & 3 deletions Framework/Kernel/test/EnumeratedStringTest.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,12 @@ class EnumeratedStringTest : public CxxTest::TestSuite {

// test constructor from enumerator
COOLGUY dude1(CoolGuys::Fred);
TS_ASSERT_EQUALS(dude1, "Frederic");
TS_ASSERT_EQUALS(dude1, CoolGuys(0));
TS_ASSERT_EQUALS(dude1, CoolGuys::Fred);
TS_ASSERT_EQUALS(dude1, coolGuyNames[0]);
TS_ASSERT_DIFFERS(dude1, CoolGuys::Bill);
TS_ASSERT_DIFFERS(dude1, "Jeremy");

// test constructor from string name
COOLGUY dude2(coolGuyNames[1]);
Expand Down Expand Up @@ -211,7 +213,7 @@ class EnumeratedStringTest : public CxxTest::TestSuite {
// try removing enum_count -- should give a compiler error
enum class Letters : size_t { a, b, enum_count };
static const std::vector<std::string> letters{"a", "b"};
enum Graphia : size_t { alpha, beta, enum_count };
enum class Graphia : size_t { alpha, beta, enum_count };
static const std::vector<std::string> graphia{"alpha", "beta"};
EnumeratedString<Letters, &letters> l1("a");
EnumeratedString<Graphia, &graphia> l2("alpha");
Expand All @@ -232,8 +234,8 @@ class EnumeratedStringTest : public CxxTest::TestSuite {
void testSwitchAndIf() {
g_log.notice("\ntestSwitchAndIf...");

const size_t index = 3;
CAKE tasty = Cakes(index), scrumptious = Cakes(index);
const char index = 3;
CAKE tasty = Cakes(index);

// test enumerated string against string
if (tasty == cakeNames[index]) {
Expand Down Expand Up @@ -262,6 +264,29 @@ class EnumeratedStringTest : public CxxTest::TestSuite {
}
}

void testUnderlyingType() {
g_log.notice("\ntestUnderlyingType...");

// use a non-class enum to compare against chars
enum LetterA : char { a, enum_count };
static const std::vector<std::string> letterA = {"a"};
typedef EnumeratedString<LetterA, &letterA> LETTERA;
LETTERA a1 = a; // note that a is in namespace, vars cannot be named this
TS_ASSERT_EQUALS(a1, a); // compare against enum
TS_ASSERT_EQUALS(a1, "a"); // compares against string
TS_ASSERT_EQUALS(a1, 0); // compare against literal
TS_ASSERT_EQUALS(a1, '\x00'); // char literal at 0
TS_ASSERT_DIFFERS(a1, 'a'); // Letters::a corresponds to 0, not to 'a'= 97
TS_ASSERT_DIFFERS(a1, '\x01'); // char literal at 1
TS_ASSERT_DIFFERS(a1, 1);
switch (a1) {
case '\x00':
break;
default:
TS_FAIL("EnumeratedString in 'SWITCH' failed to match to underlying type");
}
}

void testCaseInsensitiveNameComparison() {
g_log.notice("\ntestCaseInsensitiveNameComparison...");

Expand Down
9 changes: 7 additions & 2 deletions dev-docs/source/EnumeratedString.rst
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ would be something like the following:
However, this is not allowed under C++.

The ``EnumeratedString`` objects allow for binding an ``enum class`` to a C-style static array of strings, allowing for much
The ``EnumeratedString`` objects allow for binding an ``enum`` or ``enum class`` to a vector of strings, allowing for much
of the same behavior. This allows for easy-to-read ``if`` and ``switch`` statements, as well as easy conversions and assignments
with strings from the allowed set. This further adds an additional layer of validation for string properties, in additon to the
``StringListValidator`` used in the property declaration.
Expand Down Expand Up @@ -69,6 +69,11 @@ Notice that the final element of the ``enum`` is called :code:`enum_count`. Thi
number of elements inside the ``enum``, and used for verifying compatibility with the vector of strings. A compiler error
will be triggered if this is not included.

Further, the ``enum`` *must* have elements in order from 0 to :code:`enum_count`. That is, you *CANNOT* set them like so:
.. code-block:: cpp
enum class CakeTypeEnum : char {LEMON='l', BUNDT='b', POUND='p', enum_count=3}; // NOT ALLOWED
as this will break validation features inside the class.

Notice the use of the reference operator, :code:`&cakeTypeNames`, and *not* :code:`cakeTypeNames`.

In the above code, a :code:`CAKETYPE` object can be created either from a :code:`CakeTypeEnum`, or from one of the strings
Expand Down Expand Up @@ -104,7 +109,7 @@ An example of where this might be used inside an algorithm is shown below:
void BakeCake::exec() {
// this will assign cakeType from the string property
CAKETYPE cakeType = getProperty("CakeType");
CAKETYPE cakeType = getPropertyValue("CakeType");
// logic can branch on cakeType comparing to the enum
switch(cakeType){
Expand Down

0 comments on commit c47df2f

Please sign in to comment.