nakiya
nakiya

Reputation: 14403

Is there anything wrong with this scheme for error logging?

I wrote this interface:

//The idea is to use EHPrint to construct the error msg and then raise an error or
//a warning, etc. This removes the need to implement all 3 'Raise' functions
//as taking variable param list reducing code.                                                   
class ErrorHandler                                                            
{   
    public:
        virtual void RaiseError(bool, int) = 0;                     
        virtual void RaiseWarning(int) = 0;                                   
        virtual void RaiseMsg() = 0;                                          

        __attribute__((format(printf, 1, 2)))
        virtual const char* EHPrint(const char*, ...) = 0;                    

        virtual ~ErrorHandler(){}                                             
};

Forget multi-threading issues because one object will only be used in one thread. Implementation classes can log, raise errors to user or terminate as needed. The drawback is that you have to call EHPrint before you want to raise an error, etc. Any way I can make this more beautiful?

EDIT: I'd really like to just call one function when I want to RaiseError. As of now, I have to call EHPrintf and then call RaiseError.

EDIT2: I think it is my mistake that I coupled error handling and error logging together. Will try and separate them and see.

EDIT3: I thought I should post the logger now:

class Logger                                                        
{   
    public:
        enum LogType                                                
        {   
            LT_DEBUG = 0,                                           
            LT_WARNING,                                             
            LT_ERROR,                                               
            LT_STAT,                                                
            LT_TEXT                                                 
        };                                                          

        __attribute__((format(printf, 5, 6)))                       
        virtual const char* EHLog(LogType,                          
                int,
                const char*,                                        
                int,
                const char*,                                        
                ...) = 0;                                           

        virtual ~Logger(){}                                         
};

Which clears up the code because logging is separate now.

Upvotes: 1

Views: 124

Answers (1)

Bart van Ingen Schenau
Bart van Ingen Schenau

Reputation: 15758

Most logging libraries that support multiple levels of logmessages use only one function for generating the messages and this function takes an additional parameter indicating the level of the log message.

In your structure, that could look like this:

class ErrorHandler                                                            
{   
    private:
        virtual void RaiseError(bool, int) = 0;                     
        virtual void RaiseWarning(int) = 0;                                   
        virtual void RaiseMsg() = 0;                                          
    public:
        enum LogLevel {
            ERROR,
            WARNING,
            MSG
        };

        __attribute__((format(printf, 2, 3)))
        virtual const char* EHPrint(enum LogLevel, const char*, ...) = 0;                    

        virtual ~ErrorHandler(){}                                             
};

Some additional points you have to consider:

  1. Variadic functions are not safe to use with non-POD types. If you use printf-style functions for logging, your users are restricted to logging only primitive types. A class like std::string can not be used.
  2. With the two-call setup for preparing the message and sending it, you must be prepared that users call EHPrint multiple times before calling one of the RaiseXXX functions.

Furthermore, I second the comment from DumbCoder to take a look at existing logging libraries, if only to see what they have in common in their design and/or API.

Upvotes: 1

Related Questions