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:
ordered_char.h
ordered_char.cc
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();
}
#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! :-) \$\endgroup\$