std::fstream Logger class

[Lazy Narwhal]

I like to consider myself to be a decent c++ programmer, but this seems like it should be such a simple error (and of course, I can't fix it.) My aim is to build a Logger class that has three functions:
Initialize(char *filename); Write(char *message); Shutdown(void);
and one variable:
std::fstream *m_stream;

This is what I have:
(in logger.h):

#pragma once
#include <iostream>
#include <iomanip> //ps i have no idea what this is
#include <fstream>

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

	Bool Initialize(String filename);
	void Write(String message);
	void Shutdown(void);
private:
	std::fstream *m_stream;
};

and in logger.cpp:

#include "logger.h"
Logger::Logger()
{
	m_stream = NULL;
}

Logger::~Logger()
{
}

Bool Logger::Initialize(String filename)
{
	if (!absFilename)
	{
		MessageBox(NULL, "Invalid filename passed to Logger::Initialize()", "Fatal Error", MB_OK | MB_ICONERROR);
		return;
	}
	m_stream = new std::fstream;
	m_stream->open(filename, std::ios::out); //create file if it does not exist
	m_stream->close();
	m_stream->open(filename, std::ios::in | std::ios::out); //open file (whether it originally existed or not)

	return false;
}

void Logger::Write(String message)
{
	if (m_stream->is_open())
	{
		(*m_stream) << message;
		return;
	}
	OutputDebugString("Failed to write: \"");
	OutputDebugString(message);
	OutputDebugString("\" to logger.");
}

void Logger::Shutdown()
{
	if (m_stream->is_open())
	{
		(*m_stream) << "Logger shut down successfully.";
		m_stream->close();
		return;
	}
	//logger not associated with file
	OutputDebugString("Warning: logger shut down when log file is not open.");
}

I call it like this:

//main.cpp (shortened)
//head's up: uses windows (not standard main()), but this is a c++ problem, not a windows problem.  please do not migrate.
#include "logger.h"
Logger *g_log = NULL;

INT WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, PSTR lpCmdLine, INT nCmdShow)
{
	//create logger
	g_log = new Logger();
	if (g_log = NULL)
	{
		MessageBox(NULL, "Failed to create logger class!", "Fatal Error", MB_OK | MB_ICONERROR);
		return 0;
	}

	if (!g_log->Initialize("log.txt")) //crashes right here
	{
		MessageBox(NULL, "Failed to initialize logger!", "Fatal Error", MB_OK | MB_ICONERROR);
		return 0;
	}
	//run application
	g_log->Shutdown();
	delete g_log;
	g_log = NULL;
}

It seems too simple to screw up, but any time I access m_stream, it gives me a friendly "Access Violation" message.

[Disch]

I didn't look very close but...

if (g_log = NULL)

= is assignment. You meant ==

That's probably your problem.

(Note that you don't need to check for null here anyway... as new will never return a null pointer. If it fails to allocate it will throw an exception.)



Also... why is m_stream created dynamically? You are never deleting it, so this class will leak memory.

A general rule of thumb is to not use new unless you have to. And when you do have to... use a smart pointer to manage the memory.

<unimportant opinion>Also... an "Initialize" style function is kind of "icky". The constructor should do the initialization.</unimportant opinion>