5

How should I write an efficient exception class to show the error that can be prevented by fixing the source code mistakes before run-time?

This is the reason I chose std::invalid_argument.

My exception class(not working, obviously):

class Foo_Exception : public std::invalid_argument
{
private:
    std::string Exception_Msg;
public:
    explicit Foo_Exception( const std::string& what_arg );
    virtual const char* what( ) const throw( );
};

explicit Foo_Exception::Foo_Exception( const std::string& what_arg ) // how should I write this function???
{
    Exception_Msg.reserve( 130000 );

    Exception_Msg = "A string literal to be appended, ";
    Exception_Msg += std::to_string( /* a constexpr int */ );
    Exception_Msg += /* a char from a const unordered_set<char> */;
}

const char* Foo_Exception::what( ) const throw( )
{
    return Exception_Msg.c_str( );
}

An if block that throws Foo_Exception:

void setCharacter( const char& c )
{
    if ( /* an invalid character (c) is passed to setCharacter */ )
    {
        try
        {
            const Foo_Exception foo_exc;
            throw foo_exc;
        }
        catch( const Foo_Exception& e )
        {
            std::cerr << e.what( );
            return;
        }
    }

    /*
        rest of the code
    */
}

int main( )
{
    setCharacter( '-' ); // dash is not acceptable!

    return 0;
}

As you can see, I need to concatenate Exception_Msg with a few substrings in order to form the completed message. These substrings are not only string literals but also constexpr ints and chars from a static std::unordered_set<char>. That's why I used std::string because it has the string::operator+= which is easy to work with. And needless to say, my goal is to reduce heap allocations down to only 1.

Another very important question is that where should I place the handler( the try-catch )? Inside the main() wrapping the setCharacter() or keep it inside setCharacter?

Please make a good and standard custom exception class similar to the above. Or write your own. Thanks in advance.

user229044
  • 232,980
  • 40
  • 330
  • 338
digito_evo
  • 3,216
  • 2
  • 14
  • 42
  • 1
    No, there is no way how to extract the buffer from `std::string`. You'll have to use `new` and `std::copy` as you are doing. – Quimby Nov 11 '21 at 09:35
  • AFAIK, no. If you have some idea of the right length to allocate in the first place, do that and use `append_position = stpcpy(append_position, "foo bar");` if you have that function available (GNU/BSD, or POSIX 2008). That's like using `strcat` repeatedly but without the performance downside of scanning through the existing string by using the ancient C `str...` API designed without performance in mind: [strcpy() return value](https://stackoverflow.com/a/51547604). If you don't know an appropriate size, you might want a local buffer on the stack to new/memcpy from. – Peter Cordes Nov 11 '21 at 09:35
  • 3
    XY problem. **Why** do you "have to return `char *`"? C style strings should not be necessary unless for interaction with legacy code... – DevSolar Nov 11 '21 at 09:38
  • 4
    Can you add something like `std::string error_message` as a member to `Foo_Exception`? If so you can then `return error_message.c_str()` – Marek Klein Nov 11 '21 at 09:39
  • Also relevant: [C++: Is it possible to detach the char* pointer from an std::string object?](https://stackoverflow.com/q/8699212/580083) – Daniel Langr Nov 11 '21 at 09:41
  • 1
    Why won't you define the `std::string` object as a class member of the `Foo_Exception` class? That way you won't need to deal with `new` and `delete`. Just noticed that it is the same advice as written by @MarekKlein – SomethingSomething Nov 11 '21 at 09:41
  • This appears to be an XY problem. As currently asked, it's a duplicate of some answers which say "no". But really you just need to format some strings into a `char[]` buffer you're going to return, and what you really want is a convenient way to do that. You don't have an existing `std::string`, that's only a choice this function is making. If you wanted to ask about how to do that, or more generally about the design of your `Foo_Exception` object, you could get answers that are actually helpful. – Peter Cordes Nov 11 '21 at 09:46
  • 1
    @SomethingSomething bad idea. it's exception class. also dynamic allocation for those might be bad idea as well, exeption class should not generate exceptions. – Swift - Friday Pie Nov 11 '21 at 09:46
  • Such an awful decision to close the question with an unrelated "duplicate question". While it may technically answer @digito_evo's question - it is completely unhelpful to his problem. What he is ought to do, is to store a string in the `Foo_Exception`, set proper value at creation of the exception, and simply return its `c_str()` on `what()` call. – ALX23z Nov 11 '21 at 09:47
  • @DevSolar https://en.cppreference.com/w/cpp/error/exception/what doesnt look like XY. it's predefined API – Swift - Friday Pie Nov 11 '21 at 09:47
  • @Swift-FridayPie your response is unclear. I advised to refrain from dynamic allocations and use a class member instead. It will be cleared automatically when the exception reference is gone, and is long as the exception reference is valid, the pointer returned by `.c_str()` will remain valid – SomethingSomething Nov 11 '21 at 10:13
  • @SomethingSomething Now that I looked at the function which calls what() from another class, I noticed that it has a memory leak. The char* that what() returns never gets deleted by **delete**. I have to add something like `delete str;`, right? – digito_evo Nov 11 '21 at 10:26
  • @ALX23z: Like I commented earlier to explain the closure, it's an XY problem, and what it's currently asking about is a duplicate. If the question were edited to instead ask how this exception class should format and return a C string that can be `delete`ed, we could reopen it, or more generally how to create a custom exception class; DevSolar suggests that the XY problem runs even deeper. – Peter Cordes Nov 11 '21 at 10:46
  • 1
    @Swift-FridayPie: To the contrary, this is **exactly** the XY problem I was expecting. Your problem is not "how to pass ownership of a std::string object", your problem is that you don't know how to create custom exceptions -- which should derive from [`std::runtime_error`](https://en.cppreference.com/w/cpp/error/runtime_error) which comes with the necessary constructors. [This question](https://stackoverflow.com/questions/41753358) showcases it nicely. – DevSolar Nov 11 '21 at 10:46
  • @SomethingSomething using 'std::string' IS dynamic allocation unless it is affected by short string allocator. but it is possible to use you idea and limit amout to actual allocation to only one by reserving memory for std::string or using custom allocator – Swift - Friday Pie Nov 11 '21 at 10:56
  • @digito_evo you should refrain from calling `new` and then you want need a `delete`. As others advised, your class should derive from `std::runtime_error` rather than `std::exception`. This is right for two reasons: 1. It already implements the error message mechanism 2. Your class will be able to be casted to `std::runtime_error`, where error messages are expected, unlike in `std::exception`. – SomethingSomething Nov 11 '21 at 10:58
  • @Swift-FridayPie using `std::string` as a local variable is a "under the hood" dynamic allocation, it is transparent to you in this case. You don't directly call `new` or `delete` at any time, whereas when the `std::string` variable dies, it will call its d'tor, which will take care of deleting its own allocated dynamic memory. It keeps your code from most memory leaks – SomethingSomething Nov 11 '21 at 11:00
  • 3
    The basic idea of an exception is that the message is created *at the point the exception is thrown*, not at the point the exception is being queried (where the environment in which the exception occurred, and all its information, is already gone). So the whole concept of having the message assembled in `.what()` is flawed. You could just use `std::runtime_error` and be done with it. If you need a custom exception class -- because you want to `catch ( Foo_Exception ... )` separately from `std::exception` -- then derive from `std::runtime_error`. Your problem does not exist in the first place. – DevSolar Nov 11 '21 at 11:01
  • @DevSolar Where should the message be generated in? If not in the what() then maybe inside the ctor of exception class? or does `std::runtime_error` provide a method for doing that? – digito_evo Nov 11 '21 at 12:26
  • @digito_evo You can write `try { throw std::runtime_error("my error message"); } catch (const std::runtime_error &e) { std::cout << e.what() << std::endl; }`. You will see that the message returned from `e.what()`, which will be printed to STDOUT, will be exactly the string you pass to the c'tor of `std::runtime_error`. So when you throw the exception, your error message should be ready to be passed to the exception's c'tor. It will create its own str copy, so don't worry about your string being local. You can write new class inheriting `runtime_error` if you like a distinct exception class – SomethingSomething Nov 11 '21 at 12:44
  • 1
    @digito_evo: At the point where your program realizes "there is no way I can continue here, because {reasons}", it then goes on to `throw Foo_Exception( "{reasons}" )`. The generating of the message is the task of the code *throwing* the exception, not the exception itself. The exception object (and the message it contains) then gets passed up the caller stack until it gets caught somewhere. – DevSolar Nov 11 '21 at 12:54
  • 1
    (ctd.) The thrower knows the exact reason for the exception, the catcher knows how to go on about it. The exception object is just a dumb message between the two. – DevSolar Nov 11 '21 at 13:02
  • @Peter Cordes I edited the question. Please write an answer if you would like to. – digito_evo Nov 12 '21 at 08:39
  • It seems you're looking for something more like an `assert` than an exception, if you want to catch it in the same function that throws it. Using an exception at all seems like an over-complication. I think DevSolar is right; you're using exceptions for a problem they weren't designed to solve, and you're shooting yourself in the foot by hard-coding a bunch of string stuff into the exception itself instead of having the throw site supply the string. This way you'll need a custom exception class for every place that can throw. You could really just `std::cerr << "stuff"` without try/catch. – Peter Cordes Nov 12 '21 at 13:12
  • If all the things you're going to format are compile-time constants, you could use the C preprocessor the stringify and paste to make a string literal with the value you want. Otherwise maybe a constexpr function using `std::array` to use an actual constexpr int, still avoiding dynamic allocation or runtime formatting of strings at all. – Peter Cordes Nov 12 '21 at 13:18
  • @Peter Cordes Good idea. You mean static_assert, right? If so then I think that your solution is closer to standard problem solving methods in software engineering. Mine is too bloated. I'm trying to show a message to the programer so that he knows he has passed a wrong argument to a function. He has to fix it in the source code. I am not sure but I'm going to try to use assert. – digito_evo Nov 12 '21 at 13:37
  • If you can change your `setCharacter` function to take a constexpr arg, then yes you can use `static_assert`. But if you want the function to work with a runtime-variable arg to set, then a compile-time check isn't always possible. I assumed that's why you were using runtime checks like if() / throw in the first place. – Peter Cordes Nov 12 '21 at 13:48
  • @Peter Cordes No that's not the reason. The reason I am using run-time checks is that the message has to include a few `char`s from a static const unordered_set. I think that it makes it impossible to evaluate the message in compile-time. – digito_evo Nov 12 '21 at 16:29
  • If you can't produce a constexpr message, you won't be able to use `static_assert`. It's literally static, evaluated only at compile-time, not compiling to any asm instructions. You could maybe adjust your plans for how detailed the message is going to be, and point the programmer at a table where they can look up something for the char they passed. – Peter Cordes Nov 12 '21 at 16:33
  • @ALX23z: IDK if you noticed, but the question is currently open and has a better title. If I hadn't closed and commented, I'm not sure the question would have got edited to get at the root of the problem the querent really wanted to solve. It turns out exceptions probably aren't even the right mechanism for the kind of error-handling they want. And it's unlikely to do future readers much good to have a question with the old title but this answer; hard to find for anyone who wanted this answer but didn't have the same XY problem in their idea how to implement it. – Peter Cordes Nov 12 '21 at 18:45
  • @Peter Cordes Based on the feedback, I wrote my own answer. Please take a look at it if you'd like to. – digito_evo Nov 13 '21 at 18:07
  • @SomethingSomething Please take a look at my own answer. – digito_evo Nov 13 '21 at 18:09
  • @PeterCordes excuse me, but this is a bizarrely confusing and roundabout way to edit post and fix question. First, we close it and claim that there is already an answer (that doesn't solve anything). Then ??? And finally question was edited. Also you have way too much hope to assume that there gonna be any future readers. The whole concept of SO and Stack Exchange doesn't exactly work as intended. Also, why is it bad to post true answers to wrong questions? Why hide them away if some readers might want to ask such questions? – ALX23z Nov 14 '21 at 17:31
  • @ALX23z: I checked meta, and cases like this have already been discussed: [How to respond to XY problem when Y is an already answered (duplicate) question?](https://meta.stackoverflow.com/q/349383) - not a highly popular question so it's not a strong consensus, but from 2017 so still recent enough to be relevant. The answer concludes with "*So, as far as I'm concerned, **the priority in this kind of situation is to get the question closed**. It's well and good to add comments to the question to guide the person toward understanding their q...*" although Peter suggests they should ask a new Q. – Peter Cordes Nov 14 '21 at 18:20
  • @ALX23z: OTOH, in this case there was enough information to see some of the scope of the XY problem and get a somewhat useful answer which still amounted to formatting the string during construction of the exception, rather than in `.what()`. I still think it was a good idea to close the question until the querent had a chance to consider comments and ask about what they were really trying to do, to make sure they realize that what they asked wasn't the right question, and that further answers which directly try to answer that don't come in. You can ask on meta if you think that was wrong. – Peter Cordes Nov 14 '21 at 18:26
  • @ALX23z Your discussions together with the question being closed served me in a good way and helped me solve my problem in a very simple way that I couldn't think of before. I had implemented an unnecessarily complicated solution for a simple problem. I got to the root of the problem and started refactoring from there. The result is desirable for me. – digito_evo Nov 14 '21 at 19:56

2 Answers2

4

Rather than new char[] every time, why not have a std::string data member?

class Foo_Exception : public std::exception /* ? */
{
    std::string msg;

public:
    Foo_Exception() {
        msg.reserve( 130000 );

        msg = "A string literal to be appended, ";
        msg += "another string, ";
        msg += "yet another one";
    }
    const char * what() const noexcept override { return msg.data(); }
    /* other members ? */
}

Or you could derive from one of the std exception types that takes a string as a parameter on construction

class Foo_Exception : public std::runtime_error /* or logic_error */
{
public:
    Foo_Exception() : runtime_error("A string literal to be appended, " "another string, " "yet another one") {}
    /* other members ? */
}

However if you just have string literals, you can split them on multiple source lines, and have zero allocations.

const char * what() const noexcept override { 
    return "A string literal to be appended, "
           "another string, "
           "yet another one";
}
Caleth
  • 52,200
  • 2
  • 44
  • 75
  • 2
    OP states the function must return `char*`. – Quimby Nov 11 '21 at 09:35
  • This function belongs to a custom exception class that inherits from `std::exception` and so it needs to return a const char*. I don't know if that would be a proper overloaded function if it returned a std::string. – digito_evo Nov 11 '21 at 09:43
  • @digito_evo Just a note, you cannot overload functions which differ only in their return values. – Quimby Nov 11 '21 at 09:44
  • 1
    @Caleth why not `.c_str()`? Does `.data()` do the same? – SomethingSomething Nov 11 '21 at 09:44
  • 2
    @SomethingSomething they are synonyms (in a `const` context) – Caleth Nov 11 '21 at 09:45
  • They are synonyms in c++11, `.data()` does not guarantee that it ends with `\0` in previous versions. – Marek Klein Nov 11 '21 at 09:49
  • 1
    @SomethingSomething: Post C++11, they're synonyms. They both exist because the standard didn't used to require that `.data()` returned a 0-terminated string, meaning C++03 implementations could delay the work of maintaining a 0 terminator until/unless .c_string was called. But since that wasn't a good implementation choice for some performance reasons (e.g. writing at all is best done with other writes), and also it sucks for `.c_str` to not be `const`, they changed that. [string c\_str() vs. data()](https://stackoverflow.com/q/194634) – Peter Cordes Nov 11 '21 at 09:50
  • @MarekKlein it's 2021. C++11 (or later) has been the default for most implementations for a long while – Caleth Nov 11 '21 at 09:50
  • @Caleth: Yup, that's why the history is mostly relevant for explaining why both member functions exist at all, instead of just having one. `.c_str()` would probably be more idiomatic in this function, though, since you do want a C-string. Then future readers of this code don't have to remind themselves of the fact that `.data()` does the same thing. – Peter Cordes Nov 11 '21 at 09:53
  • 1
    `std::runtime_error` has all the plumbing you need. Don't reinvent the wheel. – DevSolar Nov 11 '21 at 11:03
  • @DevSolar could also be `std::logic_error`, or OP may not want things that catch those to catch this – Caleth Nov 11 '21 at 11:29
  • `std::logic_error` has the same tooling in place that `std::runtime_error` has. And if OP would not want generic catches to catch `Foo_Exception` as well (which is the whole point of *having* exception object hierarchy), why bother about the return type? The way to ensure more generic catches never see `Foo_Exception` is not breaking the hierarchy, but placing `catch ( Foo_Exception ... )` where appropriate so they don't get passed through to more generic handlers... – DevSolar Nov 11 '21 at 12:05
  • @DevSolar my point is that which exception class (if any) to derive from is a design decision. It might be the case that "`Foo_Exception` is not a `std::runtime_error`" and "`Foo_Exception` is not a `std::logic_error`" are both true in the OP's circumstance. – Caleth Nov 11 '21 at 12:08
  • @Caleth: I can't think of any case where you would **not** want to derive from either `runtime_error` **or** `logic_error` and **still** derive from `exception`. If you don't want your exceptions to be caught by standard handlers you could go with your very own hierarchy where you can pick the return type, and indeed the exception object type, however you like. You could well throw a `std::string` directly in that case, fugly as that may be. – DevSolar Nov 11 '21 at 12:59
  • @Caleth I finally wrote my own answer based on your answer. Please take a look at it if you wouldn't mind. – digito_evo Nov 13 '21 at 18:11
1

I solved this problem based on the feedback that I received from others through comments and answers. So I decided to leave my own answer/solution here for future readers.

Below can be seen what I came up with after much thought and research. This solution is fairly simple and readable.

Here is the exception class interface:

Foo_Exception.h

#include <exception>


class Foo_Exception : public std::invalid_argument
{
public:
    explicit Foo_Exception( const std::string& what_arg );
};

And here is its implementation:

Foo_Exception.cpp

#include "Foo_Exception.h"


Foo_Exception::Foo_Exception( const std::string& what_arg )
: std::invalid_argument( what_arg )
{
}

A function that throws Foo_Exception:

Bar.cpp

#include "Bar.h"
#include "Foo_Exception.h"


void setCharacter( const char& c )
{
    if ( /* an invalid character (c) is passed to setCharacter */ )
    {
        std::string exceptionMsg;
        exceptionMsg.reserve( 130000 );

        exceptionMsg = "A string literal to be appended, ";
        exceptionMsg += std::to_string( /* a constexpr int */ );
        exceptionMsg += /* a char from a const unordered_set<char> */;

        throw Foo_Exception( exceptionMsg );
    }

    /*
        rest of the code
    */
}

How to handle the exception:

main.cpp

#include <iostream>
#include "Bar.h"
#include "Foo_Exception.h"


int main( )
{
    try
    {
        setCharacter( '-' ); // The dash character is not valid! Throws Foo_Exception.
    }
    catch ( const Foo_Exception& e )
    {
        std::cerr << e.what( ) << '\n';
    }

    return 0;
}

Summary of the Changes:

  1. Notice how there's no need for a what() function since the compiler generates it implicitly because Foo_Exception inherits from std::invalid_argument.

  2. Also, the process of creating the exception message was moved from the Foo_Exception's ctor to the body of the function setCharacter which actually throws the aforementioned exception. This way, the Foo_Exception's ctor is not responsible for creating the message. Instead, it is created in the body of the function that throws and is then passed to the ctor of Foo_Exception to initialize the new exception object.

  3. The data member std::string Exception_Msg was also removed from Foo_Exception as it wasn't needed anymore.

  4. Finally, the try-catch block was moved to main() so that it now wraps around setCharacter() and catches a Foo_Exception object that it might throw.

Final word: Any suggestions to further improve my answer is highly appreciated. Thanks a ton for all the feedback.

digito_evo
  • 3,216
  • 2
  • 14
  • 42
  • 1
    Nice. Note that you can also take advantage of polymorphism and catch your exception as `const std::invalid_argument &e`, depending on your needs – SomethingSomething Nov 14 '21 at 08:52
  • I don't quite like the `.reserve( 130000 )` in there. It's a "magic number" with no information on how you got to that number. It seems excessive given what we see is put into the exception text. And as an exception usually means termination of the program, optimizations on the exception path are usually unnecessary. It can't really be told from your example if the exception really **is** "exceptional" or you are (ab-)using the exception mechanism to indicate "normal" failure (which you shouldn't, as that could be handled more efficiently by an error return value from `setCharacter`). – DevSolar Nov 15 '21 at 07:36
  • @DevSolar 13000 is a random number. In my case, I needed 131 chars. Ignore that number. Speaking of whether my example is exceptional or not, yes you might be right. It could be handled by an error code but I didn't want to work with codes. – digito_evo Nov 15 '21 at 07:51