Implementation of an Ordered Character Class

Question

Overview

The code below 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.

As I was writing the class I added some additional methods and behaviors that I expected to want / need in the future including the ability to either clamp or wrap-around when the end of the list of characters is reached.

There are three files included in this post:

1. ordered_char.h
2. ordered_char.cc
3. test_main.cc

The first two define and implement the class and the third provides examples of usage and test cases.

Review Request

I would like to have this code reviewed for general feedback, naming of methods / variables, implementation of pre- and post- increment / decrement operators, am I making appropriate use of exceptions.

In particular I would like to focus on ordered_char.h and ordered_char.cc in this review.

I am also looking for feedback and suggestions for the areas of this code that I am not currently happy with:

• I feel like there is too much code duplication in the implementation of the two constructors. I thought about using a delegated constructor, but I couldn't figure out how to do that properly without putting the implementation in the header.
• In general I feel like there is too much repetition in the implementation when it comes to checking and implementing behavior at the ends of the provided character string.
• The object is only valid if it has a non-empty list. This forced me to make the default list a single character. This seems like not the right behavior. I would like to force the objects to always be valid without needing to have a "non-sense" default value. I considered forcing one of the constructors to be used to create an object of this class, but then that led to a bunch of additional complexity such as not being able to declare a variable of this class type until I know what the contents of the class are.

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 ordered_char.cc 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
*/
#include <cstddef>
#include <string>
#include <string_view>
namespace BurningLogic {
/** \class OrderedChar
 The OrderedChar class provides a character than can be incremented or decremented from a
 provided discrete ordered set 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.
*/
class OrderedChar {
 public:
 /**
 * Constructor for OrderedChar objects. Requires specifing an ordered list of possible
 * characters. If two paramaters are specified the second paramater determines if the 
 * values wrap-around at the extremes (clamp = false, the default) or are clamped to the
 * first or last value (clamp = true). The default value of the OrederedChar object is
 * the first value of char_string (char_string[0]).
 * @param char_string a string containing, in order, the representable characters
 * @param clamp false for values to wrap around at the extremes or false to clamp at the extremes
 * @exception std::invalid_argument char_string is empty
 * @exception std::invalid_argument char_string contains duplicate characters
 */
 OrderedChar(std::string_view char_string, bool clamp = false);
 /**
 * Constructor for OrderedChar objects. Requires specifing an ordered list of possible
 * characters. If three paramaters are specified the second paramater specifies the inital
 * vale of the object and the third paramater determines if the values wrap-around at the
 * extremes (clamp = false, the default) or are clamped to the first or last value
 * (clamp = true).
 * @param char_string a string containing, in order, the representable characters
 * @param inital_char the inital value of the OrderedChar object
 * @param clamp false for values to wrap around at the extremes or false to clamp at the extremes
 * @exception std::invalid_argument char_string is empty
 * @exception std::invalid_argument char_string contains duplicate characters
 * @exception std::invalid_argument inital_char is not contained in char_string
 */
 OrderedChar(std::string_view char_string, char inital_char, bool clamp = false);
 /**
 * Returns the current value of the character.
 * @return the current value of the OrderedChar object
 */
 char 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 charater list.
 * @param c the character value to set the OrderedChar object to
 * @exception std::invalid_argument the character is not a memeber of the object's character list
 */
 void SetChar(char c);
 /**
 * Returns the predecessor to the object's current character value. If the object is at the
 * beginning of the charatcer 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.
 */
 char Pred(void) const;
 /**
 * Returns the successor to the object's current character value. If the object is at the end
 * of the charatcer 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.
 */
 char 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(char c) const;
 /**
 * Prefix increment operator. Increments the character value of the OrderedChar object.
 * If the object is at the end of the charatcer 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 charatcer 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 charatcer 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 charatcer 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);
 private:
 std::string cs_ = "0円";
 std::size_t index_ = 0;
 bool clamp_ = false;
 bool HasUniqueChars(std::string_view s) const;
 std::size_t FindCharIndex(std::string_view s, char c) const;
};
} // namespace BurningLogic
#endif

ordered_char.cc

#include "ordered_char.h"
#include <stdexcept>
namespace BurningLogic {
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";
OrderedChar::OrderedChar(std::string_view char_string, bool clamp) {
 if (char_string.length() == 0) {
 throw std::invalid_argument(empty_char_string_text);
 }
 if (HasUniqueChars(char_string)) {
 cs_ = char_string;
 } else {
 throw std::invalid_argument(invalid_char_string_text);
 }
 clamp_ = clamp;
}
OrderedChar::OrderedChar(std::string_view char_string, char inital_char, bool clamp) {
 if (char_string.length() == 0) {
 throw std::invalid_argument(empty_char_string_text);
 }
 if (HasUniqueChars(char_string)) {
 cs_ = char_string;
 } else {
 throw std::invalid_argument(invalid_char_string_text);
 }
 if (!IsMember(inital_char)) {
 throw std::invalid_argument(char_not_found_text);
 }
 index_ = FindCharIndex(char_string, inital_char);
 clamp_ = clamp;
}
char OrderedChar::GetChar(void) const {
 return cs_[index_];
}
void OrderedChar::SetChar(char c) {
 index_ = FindCharIndex(cs_, c);
}
char OrderedChar::Pred(void) const {
 std::size_t tmp_index;
 if (index_ != 0) {
 tmp_index = index_ - 1;
 } else {
 if (clamp_) {
 tmp_index = 0;
 } else {
 tmp_index = cs_.length() - 1;
 }
 }
 return cs_[tmp_index];
}
char OrderedChar::Succ(void) const {
 std::size_t tmp_index;
 if (index_ != (cs_.length() - 1)) {
 tmp_index = index_ + 1;
 } else {
 if (clamp_) {
 tmp_index = cs_.length() - 1;
 } else {
 tmp_index = 0;
 }
 }
 return cs_[tmp_index];
}
bool OrderedChar::IsMember(char c) const {
 for (auto& this_char : cs_) {
 if (this_char == c) {
 return true;
 }
 }
 return false;
}
OrderedChar& OrderedChar::operator++() {
 if (index_ != (cs_.length() - 1)) {
 ++index_;
 } else {
 if (!clamp_) {
 index_ = 0;
 }
 }
 return *this;
}
OrderedChar& OrderedChar::operator--() {
 if (index_ != 0) {
 --index_;
 } else {
 if (!clamp_) {
 index_ = cs_.length() - 1;
 }
 }
 return *this;
}
OrderedChar OrderedChar::operator++(int) {
 OrderedChar tmp = *this;
 ++*this;
 return tmp;
}
OrderedChar OrderedChar::operator--(int) {
 OrderedChar tmp = *this;
 --*this;
 return tmp;
}
bool OrderedChar::HasUniqueChars(std::string_view s) const {
 std::size_t len = s.length();
 for (std::size_t ii = 0; ii < len; ++ii) {
 char test_char = s[ii];
 for (std::size_t jj = (ii + 1); jj < len; ++jj) {
 if (s[jj] == test_char) {
 return false; // Exit early if a duplicate charater is found
 }
 }
 }
 return true;
}
std::size_t OrderedChar::FindCharIndex(std::string_view s, char c) const {
 for (std::size_t ii = 0; ii < s.length(); ++ii) {
 if (s[ii] == c) {
 return ii;
 }
 }
 throw std::invalid_argument(char_not_found_text);
}
} // namespace BurningLogic

test_main.cc

#include <iostream>
#include <stdexcept>
#include "ordered_char.h"
using namespace BurningLogic;
void Test01(void) {
 // Try OrderedChar(std::string_view char_string) constructor with empty string
 bool caught = false;
 try {
 OrderedChar oc("");
 }
 catch (std::invalid_argument& e) {
 std::cout << "Test01 PASS: Caught '" << e.what() << "' (Expected)\n";
 caught = true;
 }
 if (!caught) {
 std::cout << "Test01 FAIL\n";
 }
}
void Test02(void) {
 // Try OrderedChar(std::string_view char_string) constructor with repeating chars
 bool caught = false;
 try {
 OrderedChar oc("lcnwnciwuenuuj");
 }
 catch (std::invalid_argument& e) {
 std::cout << "Test02 PASS: Caught '" << e.what() << "' (Expected)\n";
 caught = true;
 }
 if (!caught) {
 std::cout << "Test01 FAIL\n";
 }
}
void Test03(void) {
 // Try various valid constructor combinations
 bool caught = false;
 try {
 OrderedChar oc1("abc123zyx"); // OrderedChar(char_string)
 OrderedChar oc2("zxc098vbn", '8'); // OrderedChar(char_string, inital_char)
 OrderedChar oc3("asdfghjkl", true); // OrderedChar(char_string, clamp)
 OrderedChar oc4("zxcvbnm12", false); // OrderedChar(char_string, clamp)
 OrderedChar oc5("qwertyuio", 'r',
 true); // OrderedChar(char_string, inital_char, clamp)
 OrderedChar oc6("1qaz2wsx3", 'q',
 false); // OrderedChar(char_string, inital_char, clamp)
 }
 catch (std::invalid_argument& e) {
 caught = true;
 }
 if (caught) {
 std::cout << "Test03 FAIL: One or more exceptions thrown\n";
 } else {
 std::cout << "Test03 PASS\n";
 }
}
void Test04(void) {
 // Test GetChar, SetChar, Pred, Succ, and IsMember
 bool caught = false;
 try {
 OrderedChar oc_no_clamp("abcdefghijklm", 'e');
 OrderedChar oc_clamp("abcdefghijklm", 'g', true);
 std::cout << "oc_no_clamp.GetChar() = '" << oc_no_clamp.GetChar() << "' expected 'e'\n";
 std::cout << "oc_no_clamp.IsMember('f') = " << oc_no_clamp.IsMember('f') << " expected 1\n";
 std::cout << "oc_no_clamp.IsMember('7') = " << oc_no_clamp.IsMember('7') << " expected 0\n";
 std::cout << "oc_no_clamp.SetChar('a')\n";
 oc_no_clamp.SetChar('a');
 std::cout << "oc_no_clamp.GetChar() = '" << oc_no_clamp.GetChar() << "' expected 'a'\n";
 std::cout << "oc_no_clamp.Pred() = '" << oc_no_clamp.Pred() << "' expected 'm'\n";
 std::cout << "oc_no_clamp.Succ() = '" << oc_no_clamp.Succ() << "' expected 'b'\n";
 std::cout << "oc_no_clamp.SetChar('m')\n";
 oc_no_clamp.SetChar('m');
 std::cout << "oc_no_clamp.GetChar() = '" << oc_no_clamp.GetChar() << "' expected 'm'\n";
 std::cout << "oc_no_clamp.Pred() = '" << oc_no_clamp.Pred() << "' expected 'l'\n";
 std::cout << "oc_no_clamp.Succ() = '" << oc_no_clamp.Succ() << "' expected 'a'\n";
 std::cout << "oc_clamp.GetChar() = '" << oc_clamp.GetChar() << "' expected 'g'\n";
 std::cout << "oc_clamp.IsMember('b') = " << oc_clamp.IsMember('b') << " expected 1\n";
 std::cout << "oc_clamp.IsMember('x') = " << oc_clamp.IsMember('x') << " expected 0\n";
 std::cout << "oc_clamp.SetChar('a')\n";
 oc_clamp.SetChar('a');
 std::cout << "oc_clamp.GetChar() = '" << oc_clamp.GetChar() << "' expected 'a'\n";
 std::cout << "oc_clamp.Pred() = '" << oc_clamp.Pred() << "' expected 'a'\n";
 std::cout << "oc_clamp.Succ() = '" << oc_clamp.Succ() << "' expected 'b'\n";
 std::cout << "oc_clamp.SetChar('m')\n";
 oc_clamp.SetChar('m');
 std::cout << "oc_clamp.GetChar() = '" << oc_clamp.GetChar() << "' expected 'm'\n";
 std::cout << "oc_clamp.Pred() = '" << oc_clamp.Pred() << "' expected 'l'\n";
 std::cout << "oc_clamp.Succ() = '" << oc_clamp.Succ() << "' expected 'm'\n";
 }
 catch (std::invalid_argument& e) {
 caught = true;
 }
 if (caught) {
 std::cout << "Test04 FAIL: One or more exceptions thrown\n";
 } else {
 std::cout << "Test04 No exceptions thrown\n";
 }
}
void Test05(void) {
 // Test pre/post increment/decrement for no clamp
 bool caught = false;
 try {
 OrderedChar oc_no_clamp("abcdefghijklm");
 // No clamp in the middle, decrement
 std::cout << "oc_no_clamp.SetChar('e')\n";
 oc_no_clamp.SetChar('e');
 std::cout << "oc_no_clamp.GetChar() = '" << oc_no_clamp.GetChar() << "' expected 'e'\n";
 std::cout << "oc_no_clamp--.GetChar() = '" << oc_no_clamp--.GetChar() << "' expected 'e'\n";
 std::cout << "oc_no_clamp.GetChar() = '" << oc_no_clamp.GetChar() << "' expected 'd'\n";
 std::cout << "(--oc_no_clamp).GetChar() = '" << (--oc_no_clamp).GetChar() << "' expected 'c'\n";
 std::cout << "oc_no_clamp.GetChar() = '" << oc_no_clamp.GetChar() << "' expected 'c'\n";
 // No clamp in the middle, increment
 std::cout << "oc_no_clamp.SetChar('e')\n";
 oc_no_clamp.SetChar('e');
 std::cout << "oc_no_clamp.GetChar() = '" << oc_no_clamp.GetChar() << "' expected 'e'\n";
 std::cout << "oc_no_clamp++.GetChar() = '" << oc_no_clamp++.GetChar() << "' expected 'e'\n";
 std::cout << "oc_no_clamp.GetChar() = '" << oc_no_clamp.GetChar() << "' expected 'f'\n";
 std::cout << "(++oc_no_clamp).GetChar() = '" << (++oc_no_clamp).GetChar() << "' expected 'g'\n";
 std::cout << "oc_no_clamp.GetChar() = '" << oc_no_clamp.GetChar() << "' expected 'g'\n";
 // No clamp at the beginning, decrement
 std::cout << "oc_no_clamp.SetChar('a')\n";
 oc_no_clamp.SetChar('a');
 std::cout << "oc_no_clamp.GetChar() = '" << oc_no_clamp.GetChar() << "' expected 'a'\n";
 std::cout << "oc_no_clamp--.GetChar() = '" << oc_no_clamp--.GetChar() << "' expected 'a'\n";
 std::cout << "oc_no_clamp.GetChar() = '" << oc_no_clamp.GetChar() << "' expected 'm'\n";
 std::cout << "oc_no_clamp.SetChar('a')\n";
 oc_no_clamp.SetChar('a');
 std::cout << "oc_no_clamp.GetChar() = '" << oc_no_clamp.GetChar() << "' expected 'a'\n";
 std::cout << "(--oc_no_clamp).GetChar() = '" << (--oc_no_clamp).GetChar() << "' expected 'm'\n";
 std::cout << "oc_no_clamp.GetChar() = '" << oc_no_clamp.GetChar() << "' expected 'm'\n";
 // No clamp at the end, increment
 std::cout << "oc_no_clamp.SetChar('m')\n";
 oc_no_clamp.SetChar('m');
 std::cout << "oc_no_clamp.GetChar() = '" << oc_no_clamp.GetChar() << "' expected 'm'\n";
 std::cout << "oc_no_clamp++.GetChar() = '" << oc_no_clamp++.GetChar() << "' expected 'm'\n";
 std::cout << "oc_no_clamp.GetChar() = '" << oc_no_clamp.GetChar() << "' expected 'a'\n";
 std::cout << "oc_no_clamp.SetChar('m')\n";
 oc_no_clamp.SetChar('m');
 std::cout << "oc_no_clamp.GetChar() = '" << oc_no_clamp.GetChar() << "' expected 'm'\n";
 std::cout << "(++oc_no_clamp).GetChar() = '" << (++oc_no_clamp).GetChar() << "' expected 'a'\n";
 std::cout << "oc_no_clamp.GetChar() = '" << oc_no_clamp.GetChar() << "' expected 'a'\n";
 }
 catch (std::invalid_argument& e) {
 caught = true;
 }
 if (caught) {
 std::cout << "Test05 FAIL: One or more exceptions thrown\n";
 } else {
 std::cout << "Test05 No exceptions thrown\n";
 }
}
void Test06(void) {
 // Test pre/post increment/decrement for with clamping
 bool caught = false;
 try {
 OrderedChar oc_clamp("abcdefghijklm", true);
 // Clamp in the middle, decrement
 std::cout << "oc_clamp.SetChar('e')\n";
 oc_clamp.SetChar('e');
 std::cout << "oc_clamp.GetChar() = '" << oc_clamp.GetChar() << "' expected 'e'\n";
 std::cout << "oc_clamp--.GetChar() = '" << oc_clamp--.GetChar() << "' expected 'e'\n";
 std::cout << "oc_clamp.GetChar() = '" << oc_clamp.GetChar() << "' expected 'd'\n";
 std::cout << "(--oc_clamp).GetChar() = '" << (--oc_clamp).GetChar() << "' expected 'c'\n";
 std::cout << "oc_clamp.GetChar() = '" << oc_clamp.GetChar() << "' expected 'c'\n";
 // Clamp in the middle, increment
 std::cout << "oc_clamp.SetChar('e')\n";
 oc_clamp.SetChar('e');
 std::cout << "oc_clamp.GetChar() = '" << oc_clamp.GetChar() << "' expected 'e'\n";
 std::cout << "oc_clamp++.GetChar() = '" << oc_clamp++.GetChar() << "' expected 'e'\n";
 std::cout << "oc_clamp.GetChar() = '" << oc_clamp.GetChar() << "' expected 'f'\n";
 std::cout << "(++oc_clamp).GetChar() = '" << (++oc_clamp).GetChar() << "' expected 'g'\n";
 std::cout << "oc_clamp.GetChar() = '" << oc_clamp.GetChar() << "' expected 'g'\n";
 // Clamp at the beginning, decrement
 std::cout << "oc_clamp.SetChar('a')\n";
 oc_clamp.SetChar('a');
 std::cout << "oc_clamp.GetChar() = '" << oc_clamp.GetChar() << "' expected 'a'\n";
 std::cout << "oc_clamp--.GetChar() = '" << oc_clamp--.GetChar() << "' expected 'a'\n";
 std::cout << "oc_clamp.GetChar() = '" << oc_clamp.GetChar() << "' expected 'a'\n";
 std::cout << "oc_clamp.SetChar('a')\n";
 oc_clamp.SetChar('a');
 std::cout << "oc_clamp.GetChar() = '" << oc_clamp.GetChar() << "' expected 'a'\n";
 std::cout << "(--oc_clamp).GetChar() = '" << (--oc_clamp).GetChar() << "' expected 'a'\n";
 std::cout << "oc_clamp.GetChar() = '" << oc_clamp.GetChar() << "' expected 'a'\n";
 // Clamp at the end, increment
 std::cout << "oc_clamp.SetChar('m')\n";
 oc_clamp.SetChar('m');
 std::cout << "oc_clamp.GetChar() = '" << oc_clamp.GetChar() << "' expected 'm'\n";
 std::cout << "oc_clamp++.GetChar() = '" << oc_clamp++.GetChar() << "' expected 'm'\n";
 std::cout << "oc_clamp.GetChar() = '" << oc_clamp.GetChar() << "' expected 'm'\n";
 std::cout << "oc_clamp.SetChar('m')\n";
 oc_clamp.SetChar('m');
 std::cout << "oc_clamp.GetChar() = '" << oc_clamp.GetChar() << "' expected 'm'\n";
 std::cout << "(++oc_clamp).GetChar() = '" << (++oc_clamp).GetChar() << "' expected 'm'\n";
 std::cout << "oc_clamp.GetChar() = '" << oc_clamp.GetChar() << "' expected 'm'\n";
 }
 catch (std::invalid_argument& e) {
 caught = true;
 }
 if (caught) {
 std::cout << "Test06 FAIL: One or more exceptions thrown\n";
 } else {
 std::cout << "Test06 No exceptions thrown\n";
 }
}
int main([[maybe_unused]] int argc, [[maybe_unused]] char* argv[]) {
 Test01();
 Test02();
 Test03();
 Test04();
 Test05();
 Test06();
}

• Fe2O3: "I added some additional methods and behaviors that I expected to want / need in the future" Don't... Add comment blocks describing your current thoughts for future development; messages to your future self. But don't spend time writing code for which you currently have no use. When a need for a "bell" or a "whistle" arises, that is the time to fulfill that need. Does anyone invest resources (money) buying an aquarium "just in case I feel the urge to have live fish in the apartment, one day... maybe in a few years..."??
• GrapefruitIsAwesome: @Fe2O3 That's a good point. As I've already written the code I don't think I'll remove it but I'll keep your advice in mind in the future.
• Fe2O3: It's "experience". For the present, you might choose to surround each fragment with some form of #if 0 // not yet / #endif to "turn off" those segments for the time being. (You may find yourself wrestling with some kind of conflict between code you NEED and code that's just THERE but currently serves no purpose. Live and learn... Cheers! :-)
• Toby Speight: I'd avoid "turning off" unused code like that. If it's untested, then discard it, else keep it functional (and consistent with any changes to the class). Whenever I've had to resurrect ifdeffed-out code like that after a few years, it's usually been easier to rewrite it from scratch!

Answer 1

I'm impressed by the clear documentation - well done! That's really helpful as a reviewer, and I expect it's equally helpful to actual users. There's a few typos (e.g. "default value of the OrederedChar object"), so worth a pass with the spell-checker - or if you don't have one that's C++-aware, a good human proof-reader.

This might be excessive:

 /** ... It is an error to
 * specify a character that is not part of the object's charater list.
 * ... 
 * @exception std::invalid_argument the character is not a memeber of the object's character list

(and there's a couple of typos in that extract, too).

---

You could compile with more warnings enabled. My set (for GCC 14) includes -Wall -Wextra -Wwrite-strings -Wno-parentheses -Wpedantic -Warray-bounds -Wmissing-braces -Wconversion -Wuseless-cast -Weffc++. Your code has no warnings with these additional checks turned on. It's even clean when using -fanalyzer (though that's arguably a bit too expensive to use for every build).

---

Interface

The class only supports one character type. It would be more flexible if it were templated on the character type, as std::string and std::string_view are.

The first constructor can be called with a single argument, but we don't want it to participate as a default conversion, so we should mark it explicit.

I don't like optional booleans that change behaviour - it's not very obvious at the call site what effect the argument has. I would prefer to see a modifier function instead, perhaps

void SetClamp(bool clamp);

---

Implementation

Constructors should use initialisers for their members, rather than assigning after initialisation. That's pretty easy here:

OrderedChar::OrderedChar(std::string_view char_string, bool clamp)
 : cs_{char_string},
 clamp_{clamp}
{
 if (cs_.empty()) {
 throw std::invalid_argument(empty_char_string_text);
 }
 if (!HasUniqueChars()) {
 throw std::invalid_argument(invalid_char_string_text);
 }
}
OrderedChar::OrderedChar(std::string_view char_string, char inital_char, bool clamp)
 : cs_{char_string},
 index_{FindCharIndex(cs_, inital_char)},
 clamp_{clamp}
{
 if (cs_.empty()) {
 throw std::invalid_argument(empty_char_string_text);
 }
 if (!HasUniqueChars(cs_)) {
 throw std::invalid_argument(invalid_char_string_text);
 }
}

We could remove the duplication in those constructor bodies quite easily - even more so if we're less picky about exactly which exception is thrown for empty string input:

OrderedChar::OrderedChar(std::string_view char_string, bool clamp)
 : OrderedChar{char_string,
 char_string.at(0), // throws std::out_of_range
 clamp}
{
}

The strings used for exception messages can all be const, and should probably be in an anonymous namespace rather than your project namespace (so they are not visible to the linker).

HasUniqueChars() and FindCharIndex() don't use anything from this, so should be static. But after improving the constructors above, they are both always called with cs_ as first argument, so instead we could remove that argument, and always get it via the this pointer:

bool OrderedChar::HasUniqueChars() const
{
 std::map<char,std::size_t> histogram;
 for (auto c: cs_) {
 if (++histogram[c] > 1) {
 return false;
 }
 }
 return true;
}
std::size_t OrderedChar::FindCharIndex(char c) const
{
 auto index = cs_.find(c);
 if (index == cs_.npos) {
 throw std::invalid_argument(char_not_found_text);
 }
 return index;
}

Logic is duplicated in ++ and Succ(), and also in -- and Pred(). We could write a pair of private functions to reduce the duplication:

std::size_t OrderedChar::PrevIndex() const
{
 if (index_ != 0) {
 return index_ - 1;
 } else if (clamp_) {
 // unchanged
 return index_;
 } else {
 // wrap around
 return (cs_.length() - 1);
 }
}
std::size_t OrderedChar::NextIndex() const
{
 if (index_ != cs_.length() - 1) {
 return index_ + 1;
 } else if (clamp_) {
 // unchanged
 return index_;
 } else {
 // wrap around
 return 0;
 }
}
char OrderedChar::Pred(void) const
{
 return cs_[PrevIndex()];
}
char OrderedChar::Succ(void) const
{
 return cs_[NextIndex()];
}
OrderedChar& OrderedChar::operator++()
{
 index_ = NextIndex();
 return *this;
}

There's no need to loop in IsMember. Just use the string's find() member.

---

Tests

My biggest criticism is that the tests are not self-checking - the output needs to be inspected to see whether they all pass. But since we know what results we expect from each operation, the code can do that for us (and using one of the established unit-test libraries could save us some legwork in this regard). We should arrange for main() to return a success value only if all the tests pass.

The test functions are poorly named. Without looking, which area of functionality does Test04 exercise?

We don't need the caught variable in our tests - just move the print statements into the code blocks:

void Test03()
{
 // Try various valid constructor combinations
 try {
 ⋮
 std::clog << "Test03 PASS\n";
 }
 catch (std::invalid_argument& e) {
 std::cerr << "Test03 FAIL: Caught " << e.what() << '\n';
 }
}

Instead of marking both of main()'s arguments [[maybe_unused]], it's much simpler and clearer just to use the zero argument form int main().

• GrapefruitIsAwesome: Thank you for the review and helpful comments. I hope to consolidate changes from all of the feedback and hopefully I will post for re-review. I will be sure and spell check the documentation. A lot of those do slip by as code and spellcheckers don’t mix very well. I expect I’ll compose my documentation comments in a text editor with spell check capabilities and then paste it into the source. Agree on the additional set of warnings to enable. I’ll try and apply them to my next go round.
• GrapefruitIsAwesome: There’s a lot of good feedback here. It’s going to take a while to digest it. I’ll probably wait to make any changes until after I’ve decided how to address the interface updates, particularly the suggestion to make the type a template parameter. I agree with your comments on the testing. These are usual an afterthought unfortunately and it show. I’m actually not familiar with any unit-test tools. It might be worth it for me to investigate.
• Toby Speight: I recommend you read about Test-Driven Development and give that a try sometime. I'm not saying it suits every circumstance, but if you have some experience with a methodology like that, you can decide when it's a good fit for your problem and when it's not. I certainly find that it helps me write better tests and better code.

Answer 2

Toby already gave a great answer. I'll just add the following:

Consider making the character list a template parameter

After construction cs_ is never changed anymore. It still takes up space though. It is possible to make this a template parameter instead (see this StackOverflow post).

Move the character list itself into its own class

If you can't make the character list a template parameter, it would still be possible to reuse a given list for multiple OrderedChar objects. A straightforward way to do this is to create a class CharacterList that just stores cs_, and some member functions to convert indices to characters from that list, and vice versa. Then OrderedChar could store a reference to a CharacterList.