6
\$\begingroup\$

I am doing a code review and I have encountered a class in an application that is throwing an exception in the constructor:

class QueueMessage
{
private:
    std::string m_bucketName;
    std::string m_objectName;

public:
    QueueMessage(const std::string& messageIn)
    {
        std::stringstream ss;
        ss << messageIn;
        PTree pt;
        json::read_json(ss, pt);
        m_bucketName = pt.get<std::string>("bucket");
        m_objectName = pt.get<std::string>("path");
        if (m_bucketName.empty() || m_objectName.empty())
        {
            throw QueueMessageException("Empty fields in queue message");
        }
    }

    std::string getBucketName() const { return m_bucketName; }
    std::string getObjectName() const { return m_objectName; }
};

The class is used to store the message from the queue (that is a JSON-like text) in its members. Is it ok to throw an exception in the constructor if the members of the class are string, int, vector, smart pointers, etc.?

\$\endgroup\$
5
  • \$\begingroup\$ Yes. In fact, handling construction problems was one of the major motivators behind adding exception handling to C++ in the first place. \$\endgroup\$ Aug 6 '14 at 13:00
  • 2
    \$\begingroup\$ @JerryCoffin That sounds like an answer... why is it a comment? Same goes for the comment below. \$\endgroup\$
    – Pimgd
    Aug 6 '14 at 13:01
  • \$\begingroup\$ So there may be problems if I use pointers (defined with *) allocated with new or malloc? \$\endgroup\$
    – sop
    Aug 6 '14 at 13:01
  • 1
    \$\begingroup\$ @sop: In that case you need to be a lot more careful. Most often, you want to use something like std::unique_ptr to manage the dynamically allocated memory. \$\endgroup\$ Aug 6 '14 at 13:03
  • 2
    \$\begingroup\$ Then post it as answer :) \$\endgroup\$
    – sop
    Aug 6 '14 at 13:11
8
\$\begingroup\$

Throwing exceptions from a constructor is not only a perfectly legitimate pattern to use. It is also necessary to correctly implement Resource Acquisition Is Initialization (RAII) for some classes (this appears to be the case in your code). And trust me, you really do want RAII :)

However, please be aware of the caveat that the destructor of the object will not be executed if the constructor throws. But the individual members of the class will be destructed.

On a related note, it is also okay for the constructor to perform work if the work is necessary for RAII. Again under the caveat that you adhere to Single Responsibility Principle (SRP) and Dependency Injection (DI). See this question: https://stackoverflow.com/questions/7048515/is-doing-a-lot-in-constructors-bad

\$\endgroup\$
1
\$\begingroup\$

Based on the comments the answer seems to be that throwing exception in constructor is not a bad practice if the members are not pointers. The problem of dynamically allocated memory management can be solved by using smart pointers, (like std::unique_ptr).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.