CODEWITHSUNDEEP
c++inheritanceabstract-class
Andy Walton
Andy Walton

Reputation: 249

Using inheritance to add functionality

I'm using an abstract base class to add logging functionality to all of my classes. It looks like this:

class AbstractLog
{
public:
    virtual ~AbstractLog() = 0;

protected:
    void LogException(const std::string &);

private:
    SingletonLog *m_log;    // defined elsewhere - is a singleton object
};

The LogException() method writes the text to the log file defined in the SingletonLog object, and then throws an exception.

I then use this as my base class for all subsequent classes (there can be hundreds/thousands of these over hundreds of libraries/DLLs).

This allows me to call LogException() wherever I would normally throw an exception.

My question is whether or not this is good design/practice.

P.S.: I'm using inheritance simply to add functionality to all of my classes, not to implement any sort of polymorphism. On top of this, the concept of all of my classes having an is-a relationship with the AbstractLog class is arguable (each class is-a loggable object? Well, yes I suppose they are, but only because I've made them so).

Upvotes: 5

Views: 475

Answers (5)

Galik
Galik

Reputation: 48615

This seems like overkill to me. And inheritance is supposed to express the is a relationship. So, in your case, you are kind of saying that every class in you project is a logger. But really you only want one logger hence the singleton.

It seems to me that the singleton gives you the opportunity to avoid both inheriting from your logger class and storing a logger class as a member. You can simply grab the singleton each time you need it.

class Logger
{
public:

    void error(const std::string& msg);
    void warning(const std::string& msg);
    void exception(const std::string& msg);

    // singleton access
    static Logger& log()
    {
        // singleton
        static Logger logger;
        return logger;
    }
};

class Unrelated
{
public:

    void func()
    {
        // grab the singleton
        Logger::log().exception("Something exceptional happened");
    }
};

I suppose I am saying it seems less obtrusive to grab your singleton through a single static method of your logger than by having every class in the project inherit from the logger class.

Upvotes: 3

DevSolar
DevSolar

Reputation: 70263

"...use this as my base class for all subsequent classes..." -- "...question is whether or not this is good design / practice."

No it is not.

One thing you usually want to avoid is multiple inheritance issues. If every class in your application is derived from some utility class, and then another utility class, and then another utility class... you get the idea.

Besides, you're expressing the wrong thing -- very few of your classes will actually be specialized loggers, right? (That is what inheritance implies -- is a.)

And since you mentioned DLLs... you are aware of the issues implied in C++ DLLs, namely the fragility of the ABI? You are basically forcing clients to use the very same compiler and -version.

And if your architecture results in (literally) hundreds of libraries, there's something very wrong anyways.

Upvotes: 0

Ed James
Ed James

Reputation: 21

I'm not sure you gain anything by having a free function to log the exception. If you have a LogException free function then presumbaly you'd also need free functions for LogError and LogWarning (to replicate Galik's functionality) and the Logger object would either be a non-local static object defined at file scope and instantiated at startup or you would need another free function (similar to the Logger method and called from all the other logging functions) in which the Logger object would be a local static object instantiated the first time the method is called. For me the Logger object captures all this neatly in one go.

Another point to consider is performance - if you're going to have a large number of objects all using a static logging object then the object could potentially struggle with a high rate of writing to the log file and your main business logic could get blocked on a file write. It might be worth considering adding the errors and warning messages to a queue inside the Logger object and then having a separate worker thread going through the queue and do the actual file writes. You would then need thread protection locks around the writing to and reading from this internal queue.

Also if you are using a static logging object then you need to be sure that the entire application including the DLLs is single threaded otherwise you could get multiple threads writing to the same file simultaneously and you'd need to put thread protection locks into the Logger even if you didn't use an internal queue.

Upvotes: 1

Robert Wadowski
Robert Wadowski

Reputation: 1427

What you suggesting will work, I think better is to create log class ( inheriting from this interface ) and use it as in a way composition ( using interface ) than inheritance - composition is weaker connection between your logic class and log class. The best practice is the less class does the better. Additional benefit is that you will be able to extend log functionality any time you want without modification of business logic.

About this singleton, maybe proxy pattern is better ?

Hope, I helped :)

Upvotes: 3

Coert Metz
Coert Metz

Reputation: 892

Except from the friend relationship, inheritance is the strongest coupling that can be expressed in C++.

Given the principle of louse coupling, high cohesion, I think it is better to use a looser type of coupling if you just want to add functionality to a class.

As the logger is a singleton making LogException a free function is the simplest way to achieve the same goal without having the strong coupling you get with inheritance.

Upvotes: 0

Related Questions