Implementation of an Ordered Character Class #2

Question

Overview

This is a follow-up review request incorporating commented from the following prior review: Implementation of an Ordered Character Class. I have incorporated many of the suggestions made by Toby Speight as well as some additional changes.

I think the outcome of the review has resulted in less code that is more flexible and easier to understand.

As before this code is my attempt a solving a problem that came up in one of my projects. I needed a type that could be constrained to a defined list of characters, be set to a specific character, and be able to be incremented / decremented to the following / proceeding character.

There are two files included in this post:

1. ordered_char.h
2. test_main.cc

The first is the header and, as this is now a class template, includes the implementation. The second file provides examples of usage and test cases.

Changes from Prior Review

I have attempted to correct all of the spelling and typo issues in the comments and documentation.

Interface

• The class is now a template on the character type can can support various character types such as char or wchar_t
• There is now only a single constructor with no optional parameters
• The constructor is marked explicit
• All of optional parameters and parameters that resulted in multiple constructors have been turned into setters
• A conversion operator has been added to allow for directly converting an object of this class into the a character of the templated type based on the current value of the object, this is similar to calling the GetChar() method
• HasUniqueChars() and FindCharIndex() were modified to act on the object instead of taking a parameter

Implementation

• The constructor uses initializers rather than assignment
• Error strings for the exceptions are now in an anonymous namespace as recommended during the review
• HasUniqueChars(), FindCharIndex(), and IsMember() were modified as recommended to make better use of library features rather than implementing my own algorithms
• Two new private methods were added NextIndex() and PrevIndex() so that redundant code could be eliminated from Pred() and Succ() as well as the pre/post increment/decrement operators

Tests

Testing is the area in which I have made the least improvement. I have reorganize the test code to be a bit cleaner and better named. New functionality has had testing added. The test code now reports PASS or FAIL for each test as well as at a summary level.

There are currently no tests for character types other than char.

I have not had the time to investigate an sort of test framework. I think I will want to do so before beginning my next project.

Review Request

I would like to have this code reviewed for general feedback, including if I have properly interpreted feedback from the prior review.

There are two c++ firsts for me in this update. This is the first class template I have written. Have I implemented it correctly? Are there additional consideration that I have missed? The second is that this is the first time I have implemented a conversion operator. Have I done so correctly? Are there any gotchas I should be aware of?

Compiling the Code

I have compiled the code on Linux with the following command:

$ g++ --version g++ (Ubuntu 13.2.0-23ubuntu4) 13.2.0 Copyright (C) 2023 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. $ g++ -std=c++17 -Wall -Wextra -Wwrite-strings -Wno-parentheses -Wpedantic -Warray-bounds -Wmissing-braces -Wconversion -Wuseless-cast -Weffc++ test_main.cc -o test_main 

No warnings or errors are reported.

ordered_char.h

#ifndef H_BURNINGLOGIC_ORDERED_CHAR #define H_BURNINGLOGIC_ORDERED_CHAR /** \file ordered_char.h \brief Header for the OrderedChar class (a header only class) */ #include <cstddef> #include <map> #include <stdexcept> #include <string> #include <string_view> namespace BurningLogic { /** \class OrderedChar * The OrderedChar class template provides a character that can be incremented or * decremented from a provided discrete ordered list of characters. The object can * optionally clamp at the ends of the provided list or wrap-around. It is an error to * instantiate an object with an empty character list or one with characters that * appear more then once. CharT is a character type. */ template <class CharT> class OrderedChar { public: /** * Constructor for OrderedChar objects. Requires specifying an ordered list of possible * characters which must be unique. The default value of the OrderedChar object is the * first value of char_string (char_string[0]). * @param char_string a string containing, in order, the representable characters * @exception std::invalid_argument char_string is empty * @exception std::invalid_argument char_string contains duplicate characters */ explicit OrderedChar(std::basic_string_view<CharT> char_string); /** * Returns the current value of the character. * @return the current value of the OrderedChar object */ CharT GetChar(void) const; /** * Sets the current value to the specified character. It is an error to * specify a character that is not part of the object's character list. * @param c the character value to set the OrderedChar object to * @exception std::invalid_argument the character is not a member of the object's character list */ void SetChar(CharT c); /** * Enables or disables clamping to the beginning or end of the character list when the object * value is decremented or incremented. If clamping is disabled then the character list will * wrap around from the beginning to end or end to beginning. The default is for clamping to be * disabled. * @param clamp true to clamp at the extremes or false for values to wrap around at the extremes */ void SetClamp(bool clamp); /** * Returns the predecessor to the object's current character value. If the object is at the * beginning of the character list and clamping is enabled it returns the first character in * the list. If the object is at the beginning of the list and clamping is not enabled then * the call will "wrap-around" and return the last character in the list. */ CharT Pred(void) const; /** * Returns the successor to the object's current character value. If the object is at the end * of the character list and clamping is enabled it returns the last character in the list. If * the object is at the end of the list and clamping is not enabled then the call will * "wrap-around" and return the first character in the list. */ CharT Succ(void) const; /** * Checks if the specified character is in the objects character list. * @param c the character value to check if in the object's character list * @return true if the character is in the object's character list and false otherwise */ bool IsMember(CharT c) const; /** * Prefix increment operator. Increments the character value of the OrderedChar object. * If the object is at the end of the character list and clamping is enabled the value of * the object is unchanged. If the object is at the end of the list and clamping is not * enabled then value will "wrap-around" and object value will be set to the first * character in the list. * @return the value of the object before it is incremented */ OrderedChar& operator++(); /** * Prefix decrement operator. Decrements the character value of the OrderedChar object. * If the object is at the beginning of the character list and clamping is enabled the value * of the object is unchanged. If the object is at the beginning of the list and clamping * is not enabled then value will "wrap-around" and object value will be set to the last * character in the list. * @return the value of the object before it is decremented */ OrderedChar& operator--(); /** * Postfix increment operator. Increments the character value of the OrderedChar object. * If the object is at the end of the character list and clamping is enabled the value of the * object is unchanged. If the object is at the end of the list and clamping is not enabled * then value will "wrap-around" and object value will be set to the first character in the * list. * @return the value of the object after it is incremented */ OrderedChar operator++(int); /** * Postfix decrement operator. Decrements the character value of the OrderedChar object. * If the object is at the beginning of the character list and clamping is enabled the value * of the object is unchanged. If the object is at the beginning of the list and clamping * is not enabled then value will "wrap-around" and object value will be set to the last * character in the list. * @return the value of the object after it is incremented */ OrderedChar operator--(int); /** * Conversion operator. Allows for implicit conversion of the object current value to * the character type is was created with. */ operator CharT() const; private: std::basic_string<CharT> cs_ = "\0"; std::size_t index_ = 0; bool clamp_ = false; bool HasUniqueChars(void) const; std::size_t FindCharIndex(CharT c) const; std::size_t NextIndex(void) const; std::size_t PrevIndex(void) const; }; } // namespace BurningLogic // Implementation follows // -------------------------------------------------------------------------------- // Anonymous namespace so these are not visible to the linker outside of this file namespace { const std::string empty_char_string_text = "char_string must be non-empty"; const std::string invalid_char_string_text = "char_string must entries must be unique"; const std::string char_not_found_text = "inital_char not found in char_string"; } // namespace namespace BurningLogic { template <class CharT> OrderedChar<CharT>::OrderedChar(std::basic_string_view<CharT> char_string) : cs_{char_string} { if (cs_.empty()) { throw std::invalid_argument(empty_char_string_text); } if (HasUniqueChars()) { cs_ = char_string; } else { throw std::invalid_argument(invalid_char_string_text); } } template <class CharT> CharT OrderedChar<CharT>::GetChar(void) const { return cs_[index_]; } template <class CharT> void OrderedChar<CharT>::SetChar(CharT c) { index_ = FindCharIndex(c); } template <class CharT> void OrderedChar<CharT>::SetClamp(bool clamp) { clamp_ = clamp; } template <class CharT> CharT OrderedChar<CharT>::Pred(void) const { return cs_[PrevIndex()]; } template <class CharT> CharT OrderedChar<CharT>::Succ(void) const { return cs_[NextIndex()]; } template <class CharT> bool OrderedChar<CharT>::IsMember(CharT c) const { return cs_.find(c) != std::basic_string<CharT>::npos; } template <class CharT> OrderedChar<CharT>& OrderedChar<CharT>::operator++() { index_ = NextIndex(); return *this; } template <class CharT> OrderedChar<CharT>& OrderedChar<CharT>::operator--() { index_ = PrevIndex(); return *this; } template <class CharT> OrderedChar<CharT> OrderedChar<CharT>::operator++(int) { OrderedChar tmp = *this; ++*this; return tmp; } template <class CharT> OrderedChar<CharT> OrderedChar<CharT>::operator--(int) { OrderedChar tmp = *this; --*this; return tmp; } template <class CharT> OrderedChar<CharT>::operator CharT() const { return cs_[index_]; } template <class CharT> bool OrderedChar<CharT>::HasUniqueChars(void) const { std::map<CharT, std::size_t> histogram; for (auto c : cs_) { ++histogram[c]; if (histogram[c] > 1) { return false; } } return true; } template <class CharT> std::size_t OrderedChar<CharT>::FindCharIndex(CharT c) const { auto index = cs_.find(c); if (index == cs_.npos) { throw std::invalid_argument(char_not_found_text); } return index; } template <class CharT> std::size_t OrderedChar<CharT>::NextIndex(void) const { if (index_ != (cs_.length() - 1)) { return index_ + 1; } else { if (clamp_) { return index_; // Clamped at end and unchanged } else { return 0; // Wraps around } } } template <class CharT> std::size_t OrderedChar<CharT>::PrevIndex(void) const { if (index_ != 0) { return index_ - 1; } else { if (clamp_) { return index_; // Clamped at beginning and unchanged } else { return cs_.length() - 1; // Wraps around } } } } // namespace BurningLogic #endif 

test_main.cc

#include <cstdlib> #include <iostream> #include <stdexcept> #include <string> #include <string_view> #include "ordered_char.h" using namespace BurningLogic; bool EvalPassFailCaseChar(std::string_view text, char test_char, char expected_char) { bool pass = (test_char == expected_char); if (pass) { std::cout << "PASS: "; } else { std::cout << "FAIL: "; } std::cout << text << " = '" << test_char << "' expected '" << expected_char << "'\n"; return pass; } std::string BoolToString(bool b) { if (b) { return "TRUE"; } else { return "FALSE"; } } bool EvalPassFailCaseBool(std::string_view text, bool test_bool, bool expected_bool) { bool pass = (test_bool == expected_bool); if (pass) { std::cout << "PASS: "; } else { std::cout << "FAIL: "; } std::cout << text << " = '" << BoolToString(test_bool) << "' expected '" << BoolToString(expected_bool) << "'\n"; return pass; } bool TestValidConstructor(void) { bool caught = false; std::cout << "\nTest with Valid Constructor\n"; std::cout << "---------------------------\n"; try { OrderedChar<char> oc("abc123zyx"); } catch (std::invalid_argument &e) { std::cout << "FAIL: Caught '" << e.what() << "' \n"; caught = true; } if (!caught) { std::cout << "PASS\n"; } return !caught; } bool TestEmptyStringConstructor(void) { bool caught = false; std::cout << "\nTest with Empty String Constructor\n"; std::cout << "----------------------------------\n"; try { OrderedChar<char> oc(""); } catch (std::invalid_argument &e) { std::cout << "PASS: Caught '" << e.what() << "' (Expected)\n"; caught = true; } if (!caught) { std::cout << "FAIL\n"; } return caught; } bool TestDuplicateCharConstructor(void) { bool caught = false; std::cout << "\nTest with Duplicate Char Constructor\n"; std::cout << "------------------------------------\n"; try { OrderedChar<char> oc("lcnwnciwuenuuj"); } catch (std::invalid_argument &e) { std::cout << "PASS: Caught '" << e.what() << "' (Expected)\n"; caught = true; } if (!caught) { std::cout << "FAIL\n"; } return caught; } bool TestMiscFunctions(void) { // Test GetChar, SetChar, Pred, Succ, and IsMember bool caught = false; char test_char; bool test_bool; bool cumulative_pass = true; std::cout << "\nMisc Tests\n"; std::cout << "----------\n"; try { OrderedChar<char> oc_no_clamp("abcdefghijklm"); std::cout << " oc_no_clamp.SetChar('e')\n"; oc_no_clamp.SetChar('e'); test_char = oc_no_clamp.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc_no_clamp.GetChar()", test_char, 'e'); test_bool = oc_no_clamp.IsMember('f'); cumulative_pass &= EvalPassFailCaseBool("oc_no_clamp.IsMember('f')", test_bool, true); test_bool = oc_no_clamp.IsMember('7'); cumulative_pass &= EvalPassFailCaseBool("oc_no_clamp.IsMember('7')", test_bool, false); oc_no_clamp.SetChar('a'); test_char = oc_no_clamp.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc_no_clamp.GetChar()", test_char, 'a'); test_char = oc_no_clamp.Pred(); cumulative_pass &= EvalPassFailCaseChar("oc_no_clamp.Pred()", test_char, 'm'); test_char = oc_no_clamp.Succ(); cumulative_pass &= EvalPassFailCaseChar("oc_no_clamp.Succ()", test_char, 'b'); oc_no_clamp.SetChar('m'); test_char = oc_no_clamp.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc_no_clamp.GetChar()", test_char, 'm'); test_char = oc_no_clamp.Pred(); cumulative_pass &= EvalPassFailCaseChar("oc_no_clamp.Pred()", test_char, 'l'); test_char = oc_no_clamp.Succ(); cumulative_pass &= EvalPassFailCaseChar("oc_no_clamp.Succ()", test_char, 'a'); OrderedChar<char> oc_clamp("qwertyuiop"); oc_clamp.SetClamp(true); std::cout << " oc_clamp.SetChar('t')\n"; oc_clamp.SetChar('t'); test_char = oc_clamp.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc_clamp.GetChar()", test_char, 't'); test_bool = oc_clamp.IsMember('r'); cumulative_pass &= EvalPassFailCaseBool("oc_clamp.IsMember('r')", test_bool, true); test_bool = oc_clamp.IsMember('a'); cumulative_pass &= EvalPassFailCaseBool("oc_clamp.IsMember('a')", test_bool, false); oc_clamp.SetChar('q'); test_char = oc_clamp.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc_clamp.GetChar()", test_char, 'q'); test_char = oc_clamp.Pred(); cumulative_pass &= EvalPassFailCaseChar("oc_clamp.Pred()", test_char, 'q'); test_char = oc_clamp.Succ(); cumulative_pass &= EvalPassFailCaseChar("oc_clamp.Succ()", test_char, 'w'); oc_clamp.SetChar('p'); test_char = oc_clamp.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc_clamp.GetChar()", test_char, 'p'); test_char = oc_clamp.Pred(); cumulative_pass &= EvalPassFailCaseChar("oc_clamp.Pred()", test_char, 'o'); test_char = oc_clamp.Succ(); cumulative_pass &= EvalPassFailCaseChar("oc_clamp.Succ()", test_char, 'p'); } catch (std::invalid_argument &e) { caught = true; } if (caught) { std::cout << "FAIL: One or more exceptions thrown\n"; cumulative_pass = false; } return cumulative_pass; } bool TestIncrementDecrementWrap(void) { bool caught = false; bool cumulative_pass = true; char test_char; std::cout << "\nTest Increment and Decrement without Clamping\n"; std::cout << "---------------------------------------------\n"; try { OrderedChar<char> oc("abcdefghijklm"); oc.SetClamp(false); // In the middle, decrement std::cout << " oc.SetChar('e')\n"; oc.SetChar('e'); test_char = oc.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc.GetChar()", test_char, 'e'); test_char = oc--.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc--.GetChar()", test_char, 'e'); test_char = oc.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc.GetChar()", test_char, 'd'); test_char = (--oc).GetChar(); cumulative_pass &= EvalPassFailCaseChar("(--oc).GetChar()", test_char, 'c'); test_char = oc.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc.GetChar()", test_char, 'c'); // In the middle, increment std::cout << " oc.SetChar('e')\n"; oc.SetChar('e'); test_char = oc.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc.GetChar()", test_char, 'e'); test_char = oc++.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc++.GetChar()", test_char, 'e'); test_char = oc.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc.GetChar()", test_char, 'f'); test_char = (++oc).GetChar(); cumulative_pass &= EvalPassFailCaseChar("(++oc).GetChar()", test_char, 'g'); test_char = oc.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc.GetChar()", test_char, 'g'); // At the beginning, decrement std::cout << " oc.SetChar('a')\n"; oc.SetChar('a'); test_char = oc.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc.GetChar()", test_char, 'a'); test_char = oc--.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc--.GetChar", test_char, 'a'); test_char = oc.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc.GetChar()", test_char, 'm'); std::cout << " oc.SetChar('a')\n"; oc.SetChar('a'); test_char = oc.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc.GetChar()", test_char, 'a'); test_char = (--oc).GetChar(); cumulative_pass &= EvalPassFailCaseChar("(--oc).GetChar()", test_char, 'm'); test_char = oc.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc.GetChar()", test_char, 'm'); // At the end, increment std::cout << " oc.SetChar('a')\n"; oc.SetChar('m'); test_char = oc.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc.GetChar()", test_char, 'm'); test_char = oc++.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc++.GetChar", test_char, 'm'); test_char = oc.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc.GetChar()", test_char, 'a'); std::cout << " oc.SetChar('a')\n"; oc.SetChar('m'); test_char = oc.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc.GetChar()", test_char, 'm'); test_char = (++oc).GetChar(); cumulative_pass &= EvalPassFailCaseChar("(++oc).GetChar", test_char, 'a'); test_char = oc.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc.GetChar()", test_char, 'a'); } catch (std::invalid_argument &e) { caught = true; } if (caught) { std::cout << "FAIL: One or more exceptions thrown\n"; cumulative_pass = false; } if (cumulative_pass) { std::cout << "PASS\n"; } else { std::cout << "FAIL: One or more cases failed\n"; } return cumulative_pass; } bool TestIncrementDecrementClamp(void) { bool caught = false; bool cumulative_pass = true; char test_char; std::cout << "\nTest Increment and Decrement with Clamping\n"; std::cout << "------------------------------------------\n"; try { OrderedChar<char> oc("abcdefghijklm"); oc.SetClamp(true); // In the middle, decrement std::cout << " oc.SetChar('e')\n"; oc.SetChar('e'); test_char = oc.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc.GetChar()", test_char, 'e'); test_char = oc--.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc--.GetChar()", test_char, 'e'); test_char = oc.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc.GetChar()", test_char, 'd'); test_char = (--oc).GetChar(); cumulative_pass &= EvalPassFailCaseChar("(--oc).GetChar()", test_char, 'c'); test_char = oc.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc.GetChar()", test_char, 'c'); // In the middle, increment std::cout << " oc.SetChar('e')\n"; oc.SetChar('e'); test_char = oc.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc.GetChar()", test_char, 'e'); test_char = oc++.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc++.GetChar()", test_char, 'e'); test_char = oc.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc.GetChar()", test_char, 'f'); test_char = (++oc).GetChar(); cumulative_pass &= EvalPassFailCaseChar("(++oc).GetChar()", test_char, 'g'); test_char = oc.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc.GetChar()", test_char, 'g'); // At the beginning, decrement std::cout << " oc.SetChar('a')\n"; oc.SetChar('a'); test_char = oc.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc.GetChar()", test_char, 'a'); test_char = oc--.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc--.GetChar", test_char, 'a'); test_char = oc.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc.GetChar()", test_char, 'a'); std::cout << " oc.SetChar('a')\n"; oc.SetChar('a'); test_char = oc.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc.GetChar()", test_char, 'a'); test_char = (--oc).GetChar(); cumulative_pass &= EvalPassFailCaseChar("(--oc).GetChar()", test_char, 'a'); test_char = oc.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc.GetChar()", test_char, 'a'); // At the end, increment std::cout << " oc.SetChar('a')\n"; oc.SetChar('m'); test_char = oc.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc.GetChar()", test_char, 'm'); test_char = oc++.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc++.GetChar", test_char, 'm'); test_char = oc.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc.GetChar()", test_char, 'm'); std::cout << " oc.SetChar('a')\n"; oc.SetChar('m'); test_char = oc.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc.GetChar()", test_char, 'm'); test_char = (++oc).GetChar(); cumulative_pass &= EvalPassFailCaseChar("(++oc).GetChar", test_char, 'm'); test_char = oc.GetChar(); cumulative_pass &= EvalPassFailCaseChar("oc.GetChar()", test_char, 'm'); } catch (std::invalid_argument &e) { caught = true; } if (caught) { std::cout << "FAIL: One or more exceptions thrown\n"; cumulative_pass = false; } if (cumulative_pass) { std::cout << "PASS\n"; } else { std::cout << "FAIL: One or more cases failed\n"; } return cumulative_pass; } bool TestConversion(void) { bool caught = false; bool cumulative_pass = true; char test_char; std::cout << "\nTest Conversion\n"; std::cout << "---------------\n"; try { OrderedChar<char> oc("abcdefghijklm"); oc.SetClamp(true); oc.SetChar('h'); test_char = oc; cumulative_pass &= EvalPassFailCaseChar("oc.GetChar()", test_char, 'h'); } catch (std::invalid_argument &e) { caught = true; } if (caught) { std::cout << "FAIL: One or more exceptions thrown\n"; cumulative_pass = false; } if (cumulative_pass) { std::cout << "PASS\n"; } else { std::cout << "FAIL: One or more cases failed\n"; } return cumulative_pass; } int main() { bool cumulative_pass = true; cumulative_pass &= TestValidConstructor(); cumulative_pass &= TestEmptyStringConstructor(); cumulative_pass &= TestDuplicateCharConstructor(); cumulative_pass &= TestIncrementDecrementWrap(); cumulative_pass &= TestIncrementDecrementClamp(); cumulative_pass &= TestMiscFunctions(); cumulative_pass &= TestConversion(); std::cout << "\nSummary\n"; std::cout << "-------\n"; if (cumulative_pass) { std::cout << "PASS: All tests passed\n"; return EXIT_SUCCESS; } else { std::cout << "FAIL: One or more tests failed\n"; return EXIT_FAILURE; } } 

Answer 1

The OrderedChar class looks pretty clean. There's an cs_ = char_string in the constructor that's clearly unnecessary (simple reasoning shows that it must already have that value) and is the only thing (apart from copyability) that prevents us making cs_ a const member.

---

The tests could be improved a bit. We embed type names in EvalPassFailCaseChar() and EvalPassFailCaseBool rather than making them overloads sharing a name. If we std::cout << std::boolalpha we could even make them instances of a single template, which would reduce the code and therefore the maintenance burden.

It's frustrating and error-prone to have to repeat ourselves like this:

 test_char = oc_no_clamp.Pred(); cumulative_pass &= EvalPassFailCaseChar("oc_no_clamp.Pred()", test_char, 'm'); 

We could use a macro to create the correct string for us:

#define TEST_EQ(expr, expected) \ EvalPassFailCase(#expr, (expr), (expected)) cumulative_pass &= TEST_EQ(oc_no_clamp.Pred(), 'm'); 

We could also use the macro to pass the file name and line number so we can report those as a prefix of any failing tests:

 std::cerr << filename << ':' << line_no << ": FAIL: " << …; 

That makes it easier for developers to jump straight to the failing test to inspect it.

I'm not a big fan of printing a line for every passing test - that becomes overwhelming very quickly and makes it easy to miss any failures. If you really want confirmation that they have all executed, just print a summary count of the number of tests that ran.

There's no need to introduce a caught variable to immediately act upon:

 catch (std::invalid_argument &e) { caught = true; } if (caught) { std::cout << "FAIL: One or more exceptions thrown\n"; cumulative_pass = false; } 

Just put the code directly into the catch block (and use the error stream for errors):

 catch (std::invalid_argument &e) { std::cerr << "FAIL: One or more exceptions thrown\n"; cumulative_pass = false; }