Singleton "Base class". Welches Design?

[Dravere]

wolke_ schrieb:

Naja, aber wenn ich ableite, dann muss ich doch explit noch alle Ctor's und operator= private machen?

Diese 4 Zeilen hast du wohl in null Komma nichts geschrieben. Also nur deswegen auf auf ein Makro setzen, würde ich nicht. Zudem hat man so auch die Möglichkeit zu wählen, ob man sie private oder protected machen möchte. Vielleicht möchte man sie sogar public machen, habe ich auch schon gesehen.

Grüssli

[wolke_]

Also ich habe mir jetzt gerade mal eine Singleton Klasse selber geschrieben. Das wichtige: Meine Singleton Klasse braucht Parameter im Constructor. Das übliche

getInstance() { static LoggerTest log; return log;}

geht bei mir also nicht.

Meine Klasse sieht so aus:

#include <cassert>
#include <string>
#include <windows.h>

class LoggerTest {
public:
	static void create(const std::string& filename, int level) {
		if(instance == NULL) {
			instance = new LoggerTest();
			instance->createImpl(filename, level);
		}
	}

	static LoggerTest* getInstance() {
		assert(instance);
		return instance;
	}

	static void destroy() {
		if(instance) {
			OutputDebugStr("close file\n");
			delete instance;
			instance = NULL;
		}
	}

	// Methods
	void message() { OutputDebugStr("message\n"); }

private:
	LoggerTest() { }
	LoggerTest(const LoggerTest& rhs);
	LoggerTest& operator=(const LoggerTest& rhs);

	static LoggerTest* instance;

	void createImpl(const std::string& filename, int level) { 
		OutputDebugStr("open file\n");
	}
};

// In Logger.cpp:
LoggerTest* LoggerTest::instance = NULL;

Die Ideen meiner Klasse: Die Klasse hat Methoden createImpl() und destroy(). createImpl() öffnet die Dateien usw. destroy() schließt die Dateien. Man benutzt meine Klasse in dem man EINMAL create(foo, bar) aufruft und dann nur noch getInstance(). Jetzt habe ich die Klasse mal getestet:

LoggerTest::create("", 2);       // ruft createImpl() auf, öffnet Dateien
LoggerTest* log = LoggerTest::getInstance();

log->message();                 // schreibt nachricht
LoggerTest::getInstance()->message();  // dito

LoggerTest::destroy();          // schließt dateien. delete instance

log->message();                 // dieser aufruft funktoniert! Wieso?
LoggerTest::getInstance()->message();

// Das hier gibt ein assert
Im Grunde steht meine Frage schon im vorletzten Kommentar. Nachdem ich destroy() aufgerufen hab, klappt log-message() dennoch! Das versteh ich nicht. Nach destroy müsste doch log auf gelöschten Speicher zeigen und dann bei log->message() einen Fehler bringen. Wieso gibt das keinen Fehler?

[drakon]

Du rufst eine Funktion auf, die den this-Zeiger nicht braucht. Somit wird da da gar nie auf den 0-Zeiger zugegriffen.

[Dravere]

wolke_ schrieb:

Wieso gibt das keinen Fehler?

Weil es undefiniertes Verhalten ist. Es wäre i.O. wenn die Welt explodiert oder deine Festplatte gelöscht wird. Es ist nicht definiert, was dann passiert, bzw. der Standard sagt eben dazu: undefined behavior.
Dies erlaubt den Kompilern unnötige Prüfungen wegzulassen, wodurch das Programm besser optimiert werden kann. Natürlich liegt dann mehr Verantwortung beim Programmierer.

Grüssli

[unskilled]

static void create(const std::string& filename, int level) { 
        if(instance == NULL) { 
            instance = new LoggerTest(); 
            instance->createImpl(filename, level); 
        } 
    }

halte ich nicht für die allerbeste möglichkeit...

ich würde das get_instance() anders implementieren und der programmierer müsste dann halt noch die init-fkt (createImpl) selbst aufrufen - aber ist wohl geschmackssache...

bb

[wolke_]

drakon schrieb:

Du rufst eine Funktion auf, die den this-Zeiger nicht braucht. Somit wird da da gar nie auf den 0-Zeiger zugegriffen.

Versteh ich nicht. 1. braucht message() einen this Zeiger und 2. zeigt log auf Müll.

@Dravere: Verstehe. Es ist also im Grunde ein Fehler. Wie könnte ich meinen Code denn ein bißchen umschreiben, dass er funktioniert bzw besser ist?

[drakon]

wolke_ schrieb:

drakon schrieb:

Du rufst eine Funktion auf, die den this-Zeiger nicht braucht. Somit wird da da gar nie auf den 0-Zeiger zugegriffen.

Versteh ich nicht. 1. braucht message() einen this Zeiger und 2. zeigt log auf Müll.

Das es undefiniert ist habe ich vergessen zu sagen, aber üblicherweise funktioniert das eben, weil kein this Zeiger gebraucht wird. Ich nehme mal nicht an, dass OutputDebugStr eine Member der Klasse ist und daher wird der this Zeiger eben nicht gebraucht..

Schlussentlich ist ja auch der -> - Operator lediglich das hier:

operator -> ( log );

Und wenn du jetzt innerhalb der Funktion log nicht benutzt, dann passiert auch nichts. (kann aber trotzdem einen Fehler geben, da undefiniert).

@Dravere: Verstehe. Es ist also im Grunde ein Fehler. Wie könnte ich meinen Code denn ein bißchen umschreiben, dass er funktioniert bzw besser ist?

Fass den Zeiger nachdem du ihn auf 0 gesetzt hast einfach nicht mehr an..

[Dravere]

drakon schrieb:

wolke_ schrieb:

@Dravere: Verstehe. Es ist also im Grunde ein Fehler. Wie könnte ich meinen Code denn ein bißchen umschreiben, dass er funktioniert bzw besser ist?

Fass den Zeiger nachdem du ihn auf 0 gesetzt hast einfach nicht mehr an..

Das ist die übliche Methode und wird auch meistens so gemacht. Allerdings gäbe es auch eine Möglichkeit über SmartPtr etwas zu machen. Du könntest zum Beispiel einen shared_ptr speichern und auch zurückgeben. Wenn du destroy aufrufst, gibst du den shared_ptr im Singleton frei, der Speicher wird aber erst freigegeben, wenn der letzte Benutzer ihn nicht mehr verwendet.
Du kannst aber auch eine eigene SmartPtr-Klasse implementieren, welche immer prüft, ob der Zeiger noch gültig ist und wenn nicht, beim Zugriff darauf eine Exception wirft.
Ein ganz einfaches Beispiel:

class LoggerTestInstance
{
public:
  LoggerTest* operator ->()
  {
    LoggerTest* instance = LoggerTest::getInstance();

    if(!instance)
    {
      throw Fehler();
    }

    return instance;
  }
};

// Verwendung:
LoggerTestInstance loggerTest;
loggerTest->message();

Kann man beliebig erweitern und verbessern

Es gibt viele weitere komplexe Wege, aber am einfachsten ist es, nach destroy nicht mehr anfassen

Grüssli

[wolke_]

Danke für die Tipps!

Dravere schrieb:

Es gibt viele weitere komplexe Wege, aber am einfachsten ist es, nach destroy nicht mehr anfassen

Ja, genau so werde ich es auch machen. Mit shared_ptr usw. find ich schon wieder mit Kanonen auf Spatzen schießen. Am Ende krieg ich sonst noch total "overdesigned" Code wie in boost und komme zu nix mehr Der Benutzer darf den Logger nach destroy() einfach nicht mehr benutzen und fertig.

[Opinion.]

Singletons are evil.