I use shared pointers because of variable can only live in block where it was created.

int main(void) {

    std::shared_ptr<project::log::Log> log;

    try {
        log = make_shared<annmu::log::Log>("error.log"); // can throw eception
    }
    catch(std::exception &e) {
        std::cout << "Error\n\n";
        return 0;
    }

}

I would like to avoid shared pointers and created code more simple. Something like following code (not working code).

int main(void) {

    project::log::Log log; // can throw eception

    try {
        log = project::log::Log("error.log"); // can throw eception
    }
    catch(std::exception &e) {
        std::cout << "Error\n\n";
        return 0;
    }

}

Is it good way to avoid shared pointers? Is it more efficient solution? In second solution the object is created two times.

Thank you for you answer.

CodePudding user response：

I would first define an interface for my logging, so I can create loggers in multiple ways (you will need this for unit testing). And then catch the exception by const reference. To be able to use the polymorphism of the interface a std::unique_ptr to that interface is needed (but at least it isn't a shared_ptr).
Full example :

#include <iostream>
#include <memory>
#include <string>

class log_itf
{
public:
    virtual void log(const std::string& str) const = 0; // logging should not modify anything
    virtual ~log_itf() = default;

protected:
    log_itf() = default;
};

class logger :
    public log_itf
{
public:
    logger(const std::string& /*filename*/)
    {
        // let this throw if it fails.
        // throw std::runtime_error("file not found");
    }

    void log(const std::string& str) const override
    {
        std::cout << str << "\n";
    }
};

// a logger that does nothing.
class null_logger :
    public log_itf
{
public:
    void log(const std::string&) const override {}
};

// example of a class that wants to use logging
class class_that_uses_logging
{
public:
    explicit class_that_uses_logging(const log_itf& logger) :
        m_logger{ logger }
    {
        m_logger.log("constructor of class that uses logging");
    }

private:
    const log_itf& m_logger;
};

// helper factory methods that do the typecasting to interface for you
std::unique_ptr<log_itf> create_logger(const std::string& filename)
{
    return std::make_unique<logger>(filename);
}

std::unique_ptr<log_itf> create_null_logger()
{
    return std::make_unique<null_logger>();
}


int main()
{
    try
    {
        // for production code
        auto logger = create_logger("log.txt");

        // for test code
        // auto logger = create_null_logger();

        logger->log("Hello World\n");

        // or pass logging to some other class
        // this pattern has a name and is called dependency
        // injection and is very useful for big projects 
        // (with unit tests)
        class_that_uses_logging object(*logger);
    }
    catch (const std::exception& e) // always catch by const&
    {
        std::cout << "error : " << e.what() << "\n";
    }

    return 0;
}

CodePudding user response：

Instead of using:

• std::shared_ptr<>
• make_shared

You could be alternatively using:

• std::unique_ptr<>
• make_unique

shared_ptr is slow and susceptible to problems.

In this case, you would need to use std::unique_ptr<>, considering that it is not needed in order to take ownership of an object while storing a pointer to another object.

In retrospect, you don't need to share ownership.

CodePudding user response：

It is always good practice to avoid using shared pointers, unless you are actually sharing the pointer. You could use a unique_ptr as a drop in replacement.
It is not a bad idea to have a non-throwing constructor, which constructs the object into a valid-empty state. Handling exceptions in construction is always more complex than handling an exception during an operation. Complex solutions require more brain-power, and brain-power is a scarce resource in large programs
So in general, I think everything you say is right. I like to think of objects as having 6 distinct stages.

• Allocation
• Construction
• Initialisation
• Active // Do useful functions, etc
• Destruction
• Deallocation

For simple objects reducing this to just construction/destruction is just convenient, and reduces how much you have to think about. For heavy-weight objects it makes sense to separate out each stage and make them separately testable. You get better error handling and error reporting this way (IMHO)