# What should a C++ error reporting exception class have to be portable across language versions? (ver 1)

## Link to revised (ver 2) question

This question contains the first version of the code for the task explained below. The revised version is at C++ exception class for error reporting, compatible with C++98 through C++20 (ver 2).

## Original (ver 1) question

I want a custom C++ exception class for error reporting I can use as a best practices model. The searches I've done so far haven't really addressed my conerns.

My specific concerns are:

1. I want a class that works well with C++98 through C++20. So far I have dealt with that using the NOEXCEPT macro. Is that sufficient? Is there a better way?

2. I want all the necessary members to work in a wide variety of circumstances, for example as part of the error reporting system of a library. Example: I have a copy constructor. Is there a compelling need to also have a move constructor for C++11 and later? Assume the exception class does not carry a large amount of data.

3. I want to have all the little details right, like exception specifications and constness.

4. Reasonable overall style, etc.

To role play a little, imagine you are using a 3rd party library that reports its errors by throwing the exception class shown below. Imagine you are using that library in a project that compiles with several different compilers (different vendors and language versions). What issues would you anticipate?

(To clarify a little, I'm not trying to go deep into the weeds with weird embedded compilers. Mostly I'm thinking of mainstream implementations like GCC, Microsoft, Intel, etc.)

I'm not too worried about exactly what standard exception class is the base, as that depends on precisely when the exception will be thrown, and I'm looking for a model that is independent of that. But I do intend to inherit std::exception at least in some cases, so if that is bad then I'd like to hear why.

FWIW, the code below compiles without complaint with GCC-8.1 -Wall -pedantic when using -std=c++98, -std=c++11, -std=c++14, -std=c++17, and -std=c++2a.

// exception.cpp
// Example of a user-defined exception class.

#include <exception>                   // std::exception
#include <iostream>                    // std::cout, std::endl
#include <sstream>                     // std::ostringstream
#include <string>                      // std::string

// A declaration specifier that says the method does not throw.
#ifndef NOEXCEPT
#  if __cplusplus >= 201103L
#    define NOEXCEPT noexcept
#  else
#    define NOEXCEPT throw()
#  endif
#endif

// Thrown in response to [application-specific circumstances].
class MyError : public std::exception {
public:      // data
// Human-readable description of this exception.
std::string m_what;

// An application-specific integer relevant to the error.  Its
// precise meaning is documented, but I have omitted that here.
// What is important is that this, along with the exception type,
// carries enough information to make error recovery decisions.
// (When appropriate, this would be an enum rather than int.)
int m_code;

// Nullable pointer to static storage containing a human-readable
// additional detail string.  There should be no reason to
// programmatically inspect it.
char const *m_detail;

private:     // methods
static std::string makeWhat(int code, char const *detail) NOEXCEPT;

public:      // methods
MyError(int code, char const *detail) NOEXCEPT;
MyError(MyError const &obj) NOEXCEPT;
virtual ~MyError() NOEXCEPT;

MyError& operator=(MyError const &obj) NOEXCEPT;
virtual char const *what() const NOEXCEPT;
};

/*static*/ std::string MyError::makeWhat(int code, char const *detail) NOEXCEPT
{
std::ostringstream oss;
if (detail) {
oss << "my error: code is " << code << ": " << detail;
}
else {
oss << "my error: code is " << code;
}
return oss.str();
}

MyError::MyError(int code, char const *detail) NOEXCEPT
: std::exception(),
m_what(makeWhat(code, detail)),
m_code(code),
m_detail(detail)
{}

MyError::MyError(MyError const &obj) NOEXCEPT
: std::exception(obj),
m_what(obj.m_what),
m_code(obj.m_code),
m_detail(obj.m_detail)
{}

MyError::~MyError() NOEXCEPT
{}

MyError& MyError::operator=(MyError const &obj) NOEXCEPT
{
if (this != &obj) {
this->std::exception::operator=(obj);
this->m_what = obj.m_what;
this->m_code = obj.m_code;
this->m_detail = obj.m_detail;
}
return *this;
}

char const *MyError::what() const NOEXCEPT
{
return m_what.c_str();
}

int main()
{
// Construct and throw a few examples.
for (int i=0; i<3; i++) {
try {
throw MyError(i, i==0? "zero" :
i==1? "one"  :
NULL);
}
catch (MyError &e) {
std::cout << e.what() << std::endl;
}
}
return 0;
}


The output of the above program is:

my error: code is 0: zero
my error: code is 1: one
my error: code is 2


Edit 1: A comment asked for additional information about the purpose of this code. The purpose is to report errors from within a library. Some example errors include:

• Error reading input from a file.
• Running out of dynamically-allocated memory.
• API misuses.

If it helps, the specific context I have in mind currently is a parsing library, but I want a model I can use in other situations as well.

Edit 2: The answer by user673679 makes good points and suggestions.

First let me acknowledge that the original code is wrong due to calling std::terminate if std::string throws; I had overlooked that. Moreover, one of the possible reasons to throw an exception is to report being out of memory, so that can't be swept under the rug.

The first proposed fix is to have what() simply return m_details, but that is not what I intend. m_details is optional supplementary information. If and when the exception is reported to the user, I want a message that combines the exception type, the information in m_code, and (when present) m_details.

The second proposed fix is to inherit std::runtime_error and have the thrower pass in the what string. But this has several problems:

1. There may be many sites that throw this exception. I do not want all of them to be responsible for constructing what. Instead, I want that centralized in one place.

2. In C++98, the std::runtime_error constructor that accepts char const * does not exist, meaning it will have to construct a std::string, and therefore possibly throw.

3. In C++11 and later, if I understand correctly, std::runtime_error will simply carry the given char const * pointer around without making a copy of the string data, but in that case, where can it be allocated? Obviously the stack is out because we're about to unwind, and the heap is no good because we could be out of memory, and even if not, there would be no opportunity to deallocate it.

The third suggestion is to use operator<<, which is appealing. The proposed implementation still uses what() internally, but I would happily dispense with that. But now the problem is my client must know to catch my exception specifically in order to know that it has an operator<<. Consequently, they cannot simply do the usual:

  catch (std::exception &e) {
cout << e.what() << endl;
}


Finally, there is Boost Error and Exception Handling page, which suggests formatting what() on demand, but I don't quite see how to do that safely since it returns a pointer rather than a std::string object. Is there an example somewhere?

• @TobySpeight I've made an edit to add additional information about the purpose. If that's not enough, could you be more specific about what is unclear? Also the meta.codereview links seem to be broken (invalid certificate). – Scott McPeak May 17 at 22:14
• The title still seems to describe your concerns about the code, rather than a description of what it's intended to achieve. The name MyError appears to be a very generic hypothetical example rather than the actual class from real working code, and there's no context showing how it's expected to be used - it would be greatly improved by showing real-world use-cases. – Toby Speight May 18 at 7:35
• The intent is an "error reporting class". I did include the concern of portability in the title because many similar questions do not have that constraint; is that your objection? As for the name, this class (albeit renamed) comes from real, working code that is about 10k LOC; I was trying to extract the salient parts (perhaps I trimmed too much). I have asked a new question with revised code and more usage details. – Scott McPeak May 18 at 9:33

• The std::string constructor and copy-constructor can throw, so we can't mark the MyError constructor or copy-constructor noexcept. More to the point, we should avoid throwing in these functions - we don't want to throw the wrong exception type at the user, or call std::terminate.

• If we want to store a what string that needs memory allocation, we might want to derive from std::runtime_error or one of the other exception classes that already implement this for us (they may use reference-counting to avoid throwing in copy-constructors).

It doesn't look like we actually need that here since the code is trivial to copy, and the detail string has static lifetime.

• Formatting the what string should be left until later. (It may never be needed, and making it displayable to a program user is the job of the catching code, not the library). We could add an operator<< for the MyError class as a simple utility.

So... I think we could just do this:

class MyError : public std::exception
{
public:

MyError(int code, char const* details) NOEXCEPT:
m_code(code),
m_details(details) { }

int code() const { return m_code; }
virtual char const* what() const NOEXCEPT { return m_details; }

private:

int m_code;
char const* m_details;
};


or this (if we want a dynamic string):

class MyError : public std::runtime_error
{
public:

MyError(int code, char const* what):
std::runtime_error(what),
m_code(code) { }

int code() const { return m_code; }

private:

int m_code;
};


and provide an operator<<:

std::ostream& operator<<(std::ostream& os, MyError const& e)
{
os << "Error [" << e.code() << "]: " << e.what();
return os;
}


The compiler will generate the necessary copy / move / destructor operations for us.

• Thanks for the feedback! You're right about std::string throwing; that is a clear bug. But your suggested fixes don't quite work for what I want; I have made an edit to the question explaining why. – Scott McPeak May 17 at 23:06

# Design review

If you really, really want to support C++98, then, to put it bluntly, you simply cannot support a best practices model. Just give it up. Literally impossible. Supporting C++98 is already the opposite of any sane definition of “best practices”.

If you want to support only C++11 and better, that’s still a challenging constraint… buuuut… you can still make something that supports best practices.

But alright, you say you want to support C++98, so I’ll review what you’ve got from that perspective. Then I’ll do the review over, this time with C++11 as the minimal supported version. You’ll see it’s COMPLETELY different; not even remotely similar.

So here goes:

## Supporting C++98

Inheriting from std::exception is one option; I’ll discuss others later. I don’t really agree with inheriting from the concrete exception types (like std::runtime_error) unless every single derived exception you will be creating fits what the base exception models. In other words, if you intend to create logic errors, don’t inherit from std::runtime_error. That seems a bit difficult to ensure, but if you’ll be disciplined, I suppose it works.

@user673679 is correct that you shouldn’t try trucking around a std::string in an exception type, due to the fact that std::string throws exceptions willy-nilly. It’s not actually a serious problem if it throws during construction. If you are constructing a MyError in a throw MyError(0, "zero"); statement, and the string construction fails and throws… well, you were already throwing an exception anyway. All that’s gone “wrong” is the second failure is being reported, rather than the first. Which… meh? I mean, at that point, you’re dealing with cascading failures, so, from an operational perspective, your program’s situation is pretty well fucked anyway. You’re still handling an error, so that’s good enough.

The real problem is that std::string can throw while copying. So, your interface would be:

class MyError : public std::exception
{
public:
MyError(int, char const*);  // can throw, no problem
MyError(MyError const&);    // can throw... BAD!

~MyError(); // cannot throw (no need to mark it noexcept)

MyError& operator=(MyError const&); // can throw... BAD!

char const *what() const NOEXCEPT;   // cannot throw
};


If std::string only threw during construction, and not during copying, then it wouldn’t be a problem.

Incidentally, insisting that detail should be a static string, and then keeping a pointer to it? Terrible idea. You can do that with some tricks in modern C++, but nothing will help you in C++98. It is inevitable that someone will screw up and use a non-static string, and then your error reporting system will need an error reporting system.

So, what can you do? Well, I recommend using a fixed-size array. Unfortunately, you can’t use std::array, because C++11, so… we’ll have to do this the ugly way (rough code, untested, etc.):

class MyError : public std::exception
{
static std::size_t const max_length = 127;

static char const what_prefix[] = "my error: code is ";

char m_what[max_length + 1];
int m_code;

public:
MyError(int code, char const* detail) NOEXCEPT
{
char* p = m_what;
char* const e = m_what + max_length;

// first, copy the "my error: code is "
std::memcpy(p, what_prefix, sizeof(what_prefix) - 1);
p += sizeof(what_prefix - 1);

// next, convert code to a string... not too difficult, but you can't
// use sprintf() (dangerous, and can fail) or snprintf() (need C++11),
// so you have to do it manually

// finally, if there's a detail message, append that
if (detail && (p + 2) > e)
{
*p++ = ':';
*p++ = ' ';

std::size_t const detail_len = std::min(std::strlen(detail), std::size_t(e - p));
std::memcpy(p, detail, detail_len);
p += detail_len;
}

// don't forget to null-terminate
*p = '\0';

m_code = code;
}

// MyError(MyError const &obj) NOEXCEPT;  // default works

// ~MyError();    // default works

// MyError& operator=(MyError const &obj) NOEXCEPT; // default works

char const *what() const NOEXCEPT
{
return m_what;
}
};


That’s all you need.

Now, is this best practices? Hell no. But this is good enough if you’re stuck with C++98.

There are other options, though.

Rather than inheriting from std::exception, you can set up your base exception type with its own hierarchy… and still eat your cake and have it, too, by also deriving from std::exception.

Here’s how:

#ifndef DEFAULT_IMPL
#   if __cplusplus >= 201103L
#       define DEFAULT_IMPL = default;
#   else
#       define DEFAULT_IMPL {}
#   endif
#endif

// define this as you please
//
// maybe add a way to get the code? you could make it pure virtual, too
struct my_error
{
virtual ~my_error() DEFAULT_IMPL
};

// this is a detail class; you should hide it from the public api
class my_error_base :
public virtual std::exception,
public virtual my_error
{
static std::size_t const max_length = 127;

char _what[_max_length];
int _code;

protected:
my_error_base() NOEXCEPT
{
_what[0] = '\0'; // or you could use a default message
}

void _init_what(char const* name, int code, char const* desc = 0) NOEXCEPT
{
// construct _what as "<name>: code is <code>: <desc>"
}
};

// now make concrete exceptions like this:

struct file_error : virtual my_error_base
{
file_error() NOEXCEPT
{
_init_what("file error", 0);
}
};

struct file_not_found : virtual file_error
{
file_not_found() NOEXCEPT
{
}
};

struct config_error : virtual my_error_base
{
config_error() NOEXCEPT
{
_init_what("config error", 0);
}
};

struct config_not_found :
virtual config_error,
virtual file_not_found
{
config_not_found() NOEXCEPT
{
}
};

// and usage:

try
{
// do configuration
//
// but:
throw config_not_found();
}
catch (file_not_found const& x)
{
}


You can catch errors with any level of specificity you like. You could catch the specific config_not_found, or more general config_error or file_not_found, or even just std::exception or your own my_error if you please. Any will work.

And you can construct errors that are more specific cases of general errors, and mix and match as you please. For example, you could create an error for a failure in some subsystem due to being out of memory, and derive it from both subsystem_error and std::bad_alloc, so you could catch either.

Is this complex and ugly? Yes. Welcome to C++98.

Is there a better way to do this? No. Not while enjoying the same flexibility and extensibility.

If I were designing an error system for C++98, this is how I’d do it. This is basically how Boost did it (though, they add a LOT of additional functionality… note, though, that they’re now suggesting not to use Boost.Exception if you’re using C++11 or better, because now there’s Boost.LEAF).

I think that’s about the best you can do with C++98. Either:

1. Derive (non-virtually) from std::exception and use that as your jumping-off base for your own hierarchy.
• PROS:
• Simple.
• Somewhat more efficient (not that that should matter; this is error-handling stuff).
• CONS:
• Inflexible. You can’t, for example, have an error in your hierarchy that derives from both MyError and, say, std::bad_alloc.
2. Derive (virtually) from std::exception and your own base error type, and then—always using virtual inheritance—derive specific exceptions from whatever you please to get as much genericity or specificity as you desire.
• PROS:
• Incredibly flexible. You can catch WHATEVER error situation you care most about in any case.
• CONS:
• Very complex. Difficult to reason about.

In either case, I’d suggest using an array data member to store the what() string, and (here I disagree strongly with @user673679) construct the what() string at construction time… NOT just when it’s requested. (This is very important if you’re debugging, and want to inspect the error object while it’s in flight, before what() is called. Unless it is just impossible to do (like for Boost.Exception exceptions), you should construct the string in the constructor.)

## C++11 or better

If you want to make a best-practices error facility for C++11 or better, this is the completely wrong way to go about it. What you should look into is std::system_error.

You don’t want to be deriving from std::exception, or from anything at all, really. (Though you could, as you’ll see.)

What you do is:

1. Create an error category for your errors.
2. Create an enum for your error codes.
3. ????
4. Profit.

Creating an error category is trivial:

// in an interface file ////////////////////////////////////////////////

auto my_error_category() noexcept -> std::error_category const&;

// in an implementation file ///////////////////////////////////////////

namespace {

class my_error_category_t : public std::error_category
{
public:
auto name() const noexcept -> const char* override
{
return "my cat"; // name it whatever you want
}

auto message(int e) const -> std::string override
{
// i'll show this in a moment
}
};

constexpr auto err_cat = my_error_category_t{};

} // anonymous namespace

auto my_error_category() noexcept -> std::error_category const&
{
return err_cat;
}


Creating an enum of error codes is also trivial. Just watch out that you don’t use zero for any error codes, for technical reasons far too deep to go into here.

// in an interface file ////////////////////////////////////////////////

enum class my_error
{
file_not_found = 1,
flux_capacitor_not_fluxing,
// ... etc.
};

namespace std {

template <>
struct is_error_code_enum<my_error> : public true_type {};

} // namespace std


Now you just need to add strings for each of the error codes:

class my_error_category_t : public std::error_category
{
public:
// ... [snip] ...

auto message(int e) const -> std::string override
{
switch (e)
{
case my_error::file_not_found:
case my_error::flux_capacitor_not_fluxing:
return "reached 88mph, but no fireworks";
// ... etc.
}
}
};


And just to make things easier:

// in an interface file ////////////////////////////////////////////////

inline auto make_error_code(my_error e) noexcept -> std::error_code
{
return std::error_code{static_cast<int>(e), my_error_category()};
}


That’s it! That’s the entirety of the error system. I mean, if you want to, you can add cross-system conversions by implementing the std::error_condition stuff (for example, my_error::file_not_found could map to std::errc::no_such_file_or_directory), but it’s not necessary.

The usage is simple:

auto main() -> int
{
for (int i = 0; i < 3; ++i)
{
try
{
if (i > 1)
throw std::system_error{make_error_code(my_error::file_not_found)};

throw std::system_error{
make_error_code(my_error::file_not_found),
i==0 ? "zero" : "one"
};
}
catch (std::system_error const& e)
{
std::cout << e.code() << '\t' << e.what() << '\n';
}
}
}

// possible output:


If you want a nicer interface, you could maybe do this:

struct my_error_t : std::system_error
{
explicit my_error_t(my_error e) :
std::system_error{make_error_code(e)}
{}

explicit my_error_t(my_error e, std::string const& msg) :
std::system_error{make_error_code(e), msg}
{}

explicit my_error_t(my_error e, char const* msg) :
std::system_error{make_error_code(e), msg}
{}
};

auto main() -> int
{
for (int i = 0; i < 3; ++i)
{
try
{
if (i > 1)
throw my_error_t{my_error::file_not_found};

throw my_error_t{my_error::file_not_found), i==0 ? "zero" : "one"};
}
catch (std::system_error const& e)
{
std::cout << e.code() << '\t' << e.what() << '\n';
}
}
}

// same output


That’s best practices for C++11 or better:

1. Do NOT make a whole new exception hierarchy.
2. Create an error category and error enum.
3. Now you can portably report errors in parallel with any other library (including the standard library, these days), and you can do so with or without exceptions (because not all errors are exceptional).

## Avoiding a fixed-size char array

It is possible to avoid a fixed-size char array in the exception class. But there are cons.

The reason I went with a fixed-size char array for my exception base class all those long eons ago is because:

• It allowed my exception code to be completely self-contained, with no dependencies on anything—not even the C standard library—which I had reasons to find desirable way, way back in the day. (These were the times where I was also writing a lot of assembly, because that’s what you had to do in the mid- to late-1990s to get real performance. It’s not true anymore.)
• It didn’t really matter that the size was limited, because the what() message isn’t really meant to give the entire story of the error that occurred. The what() message is just saying what error occurred… not where it occurred, or what the environment was at the time. For example, the what() message should be just “file not found”… it shouldn’t include the name of the file that wasn’t found. For that, your file_not_found exception class should have a filename member.

But okay, you want more space for your error messages. You can do it if you realize that it doesn’t matter if your error code throws in the exception’s constructor. Virtually all the standard library exceptions have throwing constructors. But none throw when copying.

That’s what I was getting at with the comment about std::string above: the problem isn’t that std::string might throw during construction… the problem is that it might throw during copying. If std::string were specced to be to reference counted and copy-on-write, then it would be no problem.

You could roll your own string type that is copy-on-write, but a whole dedicated type is a bit excessive for this. So you could do this:

class MyError : public std::exception
{
static char const what_prefix[] = "my error: code is ";

void _increase_refcount() NOEXCEPT
{
if (_refcount)
++refcount;
}

void _decrease_refcount() NOEXCEPT
{
if (_refcount)
{
if (--refcount == 0)
{
delete[] _what;
delete _refcount;

_what = 0;
_refcount = 0;
}
}
}

char*           _what;
std::size_t*    _refcount;
int             _code;

public:
MyError(int code, char const* detail)
{
std::size_t const len = sizeof(what_prefix - 1)
+ /* calculate the size of the stringified code */
+ (detail ? (std::strlen(detail) + 2) : 0);

_what = new (std::nothrow) char[len + 1];
_refcount = new (std::nothrow) std::size_t(1);

if (not _what or not _refcount)
{
// allocation failed, clean up and either bail with an exception,
// or fall back on a generic, static string.
}

// fill in the contents of _what

m_code = code;
}

MyError(MyError const& obj) NOEXCEPT
: _what(obj._what)
, _refcount(obj._refcount)
, _code(obj._code)
{
_increase_refcount();
}

~MyError()
{
_decrease_refcount();
}

MyError& operator=(MyError const& obj) NOEXCEPT
{
if (&obj != this)
{
_decrease_refcount();

_what = obj._what;
_refcount = obj._refcount;
_code = obj._code;

_increase_refcount();
}

return *this;
}

char const* what() const NOEXCEPT
{
if (_what)
return _what;
else
// maybe return a default static message?
}
};


This could be greatly simplified and optimized, but it shows the gist of the idea. The only failure point is the constructor, where you’re allocating the string (and the ref count). After that, there is no more allocation done… just increasing/decreasing the ref count.

## <system_error> and common practice

Unfortunately, there’s really no such thing as “common practice” when it comes to error handling in C++. It’s the wild west out there. If you use 10 different libraries in your project, you’re going to have to deal with 11 different error handling strategies.

That’s even true with the standard library. The standard library has its own custom exception hierarchy, it has errno, it has std::error_code, and probably more shit I’m forgetting.

Right now there are at least 3 different active research paths for the future of error handling in C++:

• There’s <system_error2>, which fixes the problems with <system_error>. For example, std::error_code requires std::string… which is bad for embedded systems that can’t tolerate the fact that std::string dynamically allocates memory. There’s also a problem with the way categories are identified. <system_error> uses addresses… which won’t work with complex programs that use shared libraries (the address of a category in one library may not be the same as in another).
• There’s std::expected (and its relatives, like Outcome). This is basically a type that you’d return that either contains a result, or information about why the operation failed. So if get_value() returns a std::expected<int>, you could do auto val = *get_value();, and if there was an error getting the value, this will throw an exception explaining why. Or you could do auto val = get_value(); if (val) /* use value */ else /* the op failed, so do something else */, which avoids any exceptions.
• And perhaps most interestingly, there is std::error, commonly referred to as “herbceptions” (after Herb Sutter). This would be a single exception type (rather than a hierarchy), but you could pack as much information into it as you like. It’s so efficient and simple, it could even allow passing exceptions through C code.

People like Niall Douglas are working to merge the three directions into one, unified, ultimate error handling mechanism.

However

That’s the future. Right now, the state of the art is std::error_code. It’s not common practice, because everyone just rolls their own error system… which is bad. But it is the best solution we have right now.

Unfortunately, I don’t know of any tutorials for <system_error>. But unless you want to start dabbling into error conditions and cross-system semantic comparisons, what I outlined above is pretty much the totality of what you need to know. Just make an error category, then make an enumeration of the error codes… and that’s pretty much it.

If you want to see what a library that uses the mechanism looks like, check out the standard filesystem library. For example:

bool copy_file(path const& from, path const& to);
bool copy_file(path const& from, path const& to, std::error_code& ec);


If your particular copy operation might fail, and that’s not exceptional, or if you just don’t want to deal with exceptions, you could do:

auto ec = std::error_code{};
copy_file(from, to, ec);
if (ec)
{
// deal with the error
}


On the other hand, if you do not expect the copy to fail, you could just let an exception be thrown if something really crazy happens:

copy_file(from, to);    // will either succeed, or throw a std::system_error
//
// well, technically, a std::filesystem::filesystem_error,
// which derives from std::system_error, and includes
// path info


Yes, the version that avoids exceptions is extremely clunky. That’s what things like std::expected are supposed to solve. But again, that’s the future.

• Thanks for the detailed response! For the C++98 solution, I considered an embedded char[], but dislike the fixed size limit. Still, that's better than risking a throw. For the C++11 solution, you seem to be saying not to use exception hierarchies at all, instead exclusively relying on system_error and its categories. Is that common practice? Is there a tutorial you would recommend? – Scott McPeak May 18 at 0:07
• Hm, it would take too much to answer those questions in comments, so I’ll extend the response above with the answers. – indi May 18 at 20:28
• Note that I have asked a new question with revised code. – Scott McPeak May 18 at 20:32
• Yeah, I saw, but I’m not going to be able to review anything else until later in the week at the earliest. – indi May 18 at 20:35