Simple log writer class

I've written a simple log writing class. My code is also hosted on my Github.

Specifically, I am looking for criticism on these items:

• Resource management. What is a more correct/safe way to obtaining a hold to stream-like objects?

• Clarity. Is my code as clear as it could be?

• Error handling. I've only had a few classes in C++ and every single class has only ever required me to catch std::exception. I have never needed to handle specific exceptions.

• Code organization

• Naming conventions

```
//
// Logger.h
// Kyle Shores
// 11/19/2016
//
// Defines the interface for writing data to a log file.

#include
#include
#include
#include
#include
#include
#include
#include

namespace fs = boost::filesystem;

class Logger
{
public:
Logger();
~Logger();

// all open functions return true on a successful open or true if the logger is already open
// false otherwise
bool open();
bool open(const char* pPath);
bool open(const fs::path pPath);

// write a single message
void write_log(const std::string& pMessage);
// write a group of messages
void write_log(const std::vector& pMessages);

// return true if the stream was successfully closed or if the stream was already closed
// false otherwise
bool close();

private:
bool create_and_open();

private:
fs::path mDir;
fs::path mFile;
fs::path mFull_path;
std::ofstream mStream;

// default values
std::string mDefault_dir = "./";
std::string mDefault_file = "log.log";
};

//
// Logger.cpp
// Kyle Shores
// 11/19/2016
//
// Implements the Logger class which writes data to a log file.

#include "Logger.hpp"

//=============================================================
// Public
//=============================================================

Logger::Logger()
{
}

Logger::~Logger()
{
close();
}

//==================================
// Open functions
//==================================
boo

The code is pretty good, and quite readable. Here are some of the things I noticed:

Header guards

Your Logger.h needs header guards:

#ifndef LOGGER_H_
#define LOGGER_H_

// rest of header file

#endif

Alternatively, you could use #pragma once at the top of the file, although it is theoretically less portable.

Leaking namespaces

namespace fs = boost::filesystem;

This is in the header file. That means that anyone who includes Logger.h now has a namespace fs which is boost::filesystem. This is bad, as if I am a user of your library, I would not expect it to define a fs namespace.

To fix this, move that line of code to your Logger.cpp, and use boost::filesystem in place of fs within your header file.

Namespaces

Your Logger should be in a namespace of some sort. When defining small utilities such as this, I'm usually lazy and simply place it in a utils namespace.

Invert the try in your close function

bool Logger::close()
{
    try
    {
        if (mStream.is_open())
        {
            mStream.flush();
            mStream.close();
            return !mStream.is_open();
        }
        else
            return false;
    }
    catch (std::exception& e)
    {
            std::cerr << "Error: " << e.what() <<std::endl;
            return false;
    }
}

There's a little overhead to a try-catch block; I would have expected this to look like this:

bool Logger::close()
{
    if (mStream.is_open())
    {
        try
        {
            mStream.flush();
            mStream.close();
            return !mStream.is_open();
        }
        catch (std::exception& e)
        {
            std::cerr << "Error: " << e.what() <<std::endl;
            return false;
        }
    }
    return false;
}

Only include what you need

I strongly recommend that the header file only includes what it needs, and not what the cpp file needs as well. This can improve compilation time for anyone who uses your Logger.h. I commented out the header files you don't use in your Logger.h:

#include 
// #include 
#include 
// #include 
// #include 
// #include 
#include 
#include 

Then you'd just make your Logger.cpp to look like this:

#include "Logger.hpp"

#include 
#include 
#include 
#include 

What is a logger?

The class you wrote isn't actually a logger in what people would expect. Yes, it allows you to log messages to a file with a timestamp, but most loggers out there also have something known as "logging levels", such as Debug, Warning, and Error. The idea is that you can tell the logger what level to log, and it would ignore anything which doesn't fit under that level. Your logger doesn't do this.

Good comments

Your comments are, for the most part, excellent. They mostly help with understanding the code and what the functions do, and there are not too many comments. Good job.

StackExchange Code Review Q#147882, answer score: 10