0

I'm trying to summarize some experimental results in a file. The results are generated in various C++ classes and files. I want them to all write to the same file.

For this reason it would be conventient to have a header that defines everything, and then I can just include it in the files that need to write to it. I should be a singleton, so it is not tried to open the same file twice.

It looks like this:

#ifndef FILELOGGER_H
#define FILELOGGER_H

#include <fstream>

class FileLogger{
    std::ofstream *logfile;
    static FileLogger *s_instance;

    FileLogger()
    {

        logfile = new std::ofstream();
        logfile->open("~/results/experiments.txt", std::ios_base::app);
    }

    ~FileLogger()
    {
        std::cout << "Destructor of logger called" << std::endl;
        if(s_instance)
        {
            logfile->close();
            delete logfile;
            delete s_instance;
        }
    }

public:
    static std::ofstream *instance()
    {
        if (!s_instance)
        {
            s_instance = new FileLogger();
        }

        std::cout << "got logger" << std::endl;

        return s_instance->logfile;
    }

};

FileLogger *FileLogger::s_instance = 0;



#endif // FILELOGGER_H

I would now think that in another file I just do:

#include "FileLogger.h"

and then use 

*FileLogger::instance() << "Testoutput" << std::endl;

to write to the file. However, if I try it out, the file is not created; if I create it per hand nothing is written to it. I do get the output of "got logger", that is called when the logger is accessed via the instance method. I also noticed that the destructor is never called.

Why is this not working / Is this bad style?

RunOrVeith
  • 4,487
  • 4
  • 32
  • 50
  • I will skip the part "singleton is an anti pattern" ;) What is the result of open? Your line `FileLogger *FileLogger::s_instance = 0;` should be in a cpp file – rocambille Aug 03 '16 at 11:34
  • Do your application have the write access to the file ? – The Philomath Aug 03 '16 at 11:37
  • 2
    You delete your instance in your destructor. But it's the other way round, deleting your instance should call your destructor. – Jacques de Hooge Aug 03 '16 at 11:38
  • There's no reason to use a pointer for the stream, and singletons are generally implemented as static locals returned by reference. One 2-line function and you're done. – chris Aug 03 '16 at 11:38
  • This answer may be useful in the design of your singleton: http://stackoverflow.com/a/1008289/3807729 – Galik Aug 03 '16 at 12:00
  • Ok the mistake was that ~ is not expanded, because it is a shell feature, not from the OS. Duh. It works otherwise, but I switched to the 2-liner @chris suggested. Neat trick and much less complicated! – RunOrVeith Aug 03 '16 at 12:08
  • Why the pointer for the stream? – Lightness Races in Orbit Aug 03 '16 at 12:30

2 Answers2

0

Let go of the 'header' trick. Just declare a class in a header, implement it in a cpp file, and use a static counter or static boolean to prevent multiple instantiations. I think that's much simpler than this header-only singleton magic.

Jacques de Hooge
  • 6,750
  • 2
  • 28
  • 45
  • 1
    Indeed. In fact, why bother counting it at all? Just declare as many as you need (which is one) and let it go at that. Don't be paranoid about someone sneaking into your house in the night and adding a few more throughout your code. – H. Guijt Aug 03 '16 at 12:17
  • 1
    Agreed. If you only need 2 objects of a certain class you wouldn't use a 'gemini' pattern.... Although it's said that in programming you should allow either 0, 1 or infinitely many objects... – Jacques de Hooge Aug 03 '16 at 12:31
  • But how would that fix OP's problem? – juanchopanza Aug 03 '16 at 14:53
0

This code

    logfile->open("~/results/experiments.txt", std::ios_base::app);

tries to open a file with the literal name ~/results/experiments.txt. Tilde expansion to your home directory is done by your command shell (probably bash). You need to use the actual name of your home directory, for example:

    logfile->open("/home/yourusername/results/experiments.txt", std::ios_base::app);
Andrew Henle
  • 32,625
  • 3
  • 24
  • 56