Codedesign: Logging Class

[viande]

Maddes schrieb:

namespace Log {

    Logging log = Logging::getInstance();
    //Referenzzeichen weg
}

Überladen:

void Logging::operator <<(const char* msg )
//das const ist wichtig, sonst wird dein Aufruf unten fehlschlagen
{
    // hier in datei schreiben

}

würde das schon genügen?

Funktionieren tut es auf jedenfall, aber vielleicht habe ich ja noch was nicht bedacht

int main()
{
  using namespace Log;

  log << "moin moin";

}

Echt, das hat funktioniert? :o
Aber sonst sieht das von der Idee her so aus, ja. Vielleicht sogar lieber nur using Log::log.

[Maddes]

Echt, das hat funktioniert? :o

jaja, du weisst doch was gemeint war
Passiert, wenn man an zwei Codes gleichzeitig schreibt (ich glaub das können nur Frauen...)

Jetzt noch eine Lösung zu folgenem und ich bin glücklich

void Logging::operator <<( string msg ) 
// oder const char* msg oder wie auch immer man möchte

funzt super:

log << "blabla";

aber mittels:

void Logging::operator <<( string msg ) 
...
void Logging::operator ()( int loglevel )
...

funktioniert sowas:

log(1) << "blabla";

leider nicht:

Fehler: void-Wert nicht ignoriert wie es sein sollte

Dachte mir, damit könnte man einen "Loglevel" (debug, warning, error, etc) mitschicken.

Auf jedenfall schon mal ein dickes Danke

[Konrad Rudolph]

Maddes schrieb:

void Logging::operator <<( string msg ) 
// oder const char* msg oder wie auch immer man möchte

funzt super:

log << "blabla";

Ja. Aber das hier funzt gar nicht super:

int my_value = 42;
log << "Wert = " << my_value;

Also: 'op<<' um zwei Dinge erweitern:

1. Parameter sollte ein Template sein
2. Zum Verketten muss der Log zurückgegeben werden.

template <typename T>
Logging& Logging::operator <<(T const& msg)
{
    // msg in die Datei schreiben.
    return *this;
}

/EDIT: Und für Deinen 'op()' gilt übrigens dasselbe.

[YASC]

Maddes schrieb:

Echt, das hat funktioniert? :o

void Logging::operator <<( string msg ) 
...
Logging &Logging::operator ()( int loglevel ) //referenz auf *this zurückgeben, dann sollte das weiter unten funktionieren
...

funktioniert sowas:

log(1) << "blabla";

Du musst IMHO einfach eine Referenz auf *this vom operator() zurückgeben, dann sollte das mit dem Loglevel funktionieren. (Hab da mal nicht auf const-correctness geachtet )

[Maddes]

Zum Verketten muss der Log zurückgegeben werden

einfach eine Referenz auf *this vom operator() zurückgeben

Danke euch beiden, das wars

Die Template-Idee ist jetzt die Luxusklasse

so gefällt mir das!

Danke an Alle!

[*D*Evil]

Achja ... wäre vllt ganz gut wenn du noch mit templates arbeiten würdest, um für alle dateitypen den streamoperator nutzen zu können

[volkard]

trotzdem geht an makros kein weg vorbei, wenn du vermeiden willst, daß die argumente (manchmal teure funktionsaufrufe) ausgewertet werden, wenn das eingestellte loglevel gerade sagt, daß diese zeile nicht loggen soll.

[Maddes]

trotzdem geht an makros kein weg vorbei

hmmf, ja, das durft ich grad selbst herausfinden

wenn ich zum Beispiel

log(1) << "foo " << "bar";

habe, schreibt er:

DEBUG: hallo DEBUG: bar

das ist natürlich auch Quatsch. Dort müsste man wahrscheinlich eine Flag setzten, wenn DEBUG (oder was auch immer) ausgegeben wurde, und bei einem Newline diese wieder löschen. Oder so in der Art

Da mir das aber alles noch zu unklar ist, lasse ich dieses Feature lieber ersteinmal raus.

Es ist da aber gleich noch ein Problem aufgetreten - und zwar mit dem Namesraum: (sorry für die noob-hafte Formulierung)

Beim Testen habe ich also auf eine Header verzichtet, und alles in eine test.cpp gepackt:

class Logging {
...
}

namespace Log {

  Logging log = Logging::getInstance();

}

int main() {

  Log::log << "hallo";

}

das hat so (in der Art) auch funktioniert.

Jetzt möchte ich aber die Implementierung und die Methodendefinition aber trennen, was nicht ganz klappt.

Zum Beispiel:

//class.Log.h

class Logging {
...
};

namespace Log {

  Logging log = Logging::getInstance();

}

//class.Log.cpp

#include "class.Log.h"

Logging:: ...

das will nicht recht. Auch habe ich versucht, die Klasse mit in den Namesraum zu packen.
Fehler beim Kompilieren von test.cpp : multiple definition of `Log::log'

Ich habs dann ersteinmal mit einem Makro gelöst:

#define log Logging::getInstance()

Aber weiss nicht, ob das so die geschickteste und sauberste Lösung ist

[viande]

//log.hpp
namespace Log
{
    class Logging
    {
        //...
    };

Logging log = Logging::getInstance();
}

//log.cpp
#include log.hpp
namespace Log
{
    Logging::Logging()
    {
    }
}

Das geht nicht?

[*D*Evil]

Der Sinn von dem ganzen Code ist nur fragwürdig, so wie ihr das im Moment macht ... dann kann man auch direkt globale Instanzen nehmen ...

[viande]

(D)Evil schrieb:

Der Sinn von dem ganzen Code ist nur fragwürdig, so wie ihr das im Moment macht ... dann kann man auch direkt globale Instanzen nehmen ...

Klar kann man das, aber möchtest du gerne, dass an zwei verschiedenen Stellen geloggt wird, nur weil jemand auf die Idee kommt noch eine Instanz zu erzeugen?

Alternativ wäre es auch möglich, alle Methoden und Attribute static zu machen. Find ich aber persönlich hässlich.
Bleibt noch die Möglichkeit den Konstruktor privat zu machen und eine Friend-Methode statt der getInstanze()-Methode bereit zu stellen, aber da ist die Singleton-Variante schöner.
Oder hast du noch einen Vorschlag?

Außerdem wird die Instanz im Namespace "versteckt" und sollte daher für keine Konflikte sorgen. Und es erspart einem Tipparbeit und sorgt für Übersichtlichkeit. Find ich zumindest

[*D*Evil]

Hmm du weißt das ich den Code zum Thema Singleton gepostet hab? Und ich mich mich eigentlich mit der globalen Instanz darauf bezog, das ihr hingeht, und, zwar in einem namespace, wieder eine Instanz habt. Das ist unnötig ... warum ruft ihr nicht einfach instance immer auf? Ist nun wirklich nicht soo viel aufwand.

[viande]

Ich wollte dich nicht angreifen, tut mir Leid, wenn es vielleicht so rübergekommen ist.

Worum es mir geht, ist, dass ich finde, dass die Variante mit getInstance einfach schwerer zu lesen ist. Außerdem sieht man nicht sofort, was da passiert und der Anwender der Loggingklasse muss sich darüber Gedanken machen, dass die Klasse als Singleton implementiert ist.

Ich wollte einfach nur wissen, ob du vielleicht noch eine gute Lösung für das Problem hast. Würde mich auch im Allgemeinen mal interessieren.

Logging::getInstance() << "Text";
//Hier müste ich erstmal raten, dass die Klasse vielleicht ein Singleton ist
//oder ich müsste in die Dokumentation gucken

logging::log << "Text";
//bzw
using logging::log;
log << "Text";
//Hier sieht man einfach auf einen Schlag, was passiert.

[Chris++ 0]

viande schrieb:

Außerdem sieht man nicht sofort, was da passiert und der Anwender der Loggingklasse muss sich darüber Gedanken machen, dass die Klasse als Singleton implementiert ist.

Andererseits sieht der Anwender sofort das es sich um ein Singleton handelt.

[volkardus]

Maddes schrieb:

trotzdem geht an makros kein weg vorbei

hmmf, ja, das durft ich grad selbst herausfinden
wenn ich zum Beispiel

log(1) << "foo " << "bar";

habe, schreibt er:

DEBUG: hallo DEBUG: bar

das ist natürlich auch Quatsch. Dort müsste man wahrscheinlich eine Flag setzten, wenn DEBUG (oder was auch immer) ausgegeben wurde, und bei einem Newline diese wieder löschen. Oder so in der Art

nein, da müßtest du zu tief in die stream-klassen einsteigen. eher gar nicht auf ein Newline achten, sondern ein

struct EndLogLine{
};
const EndLogLine endLogLine;

und
log(1) << "foo " << "bar" << endLogLine;

besser fühlt sich für mich an

//ungetestet
char *loglevelNames[]={"DEBUG",...};
enum Loglevel{DEBUG,...};
class LogProxy{
   ofstream file;
public:
   LogProxy(Loglevel loglevel)
   :file("log.txt"){
      file<<getTimeString()<<' '<<loglevelNames[loglevel]<<": ";
   }
   ~LogProxy(){
      file<<'\n';
   }
   friend template<typename T>
   LogProxy operator<<(T& t){
      file<<t;
      return *this;
   }
};
#define LOG(level,tolog) \
#if (level)>=(LOGLEVEL) \
  LogProxy(level)<<tolog
#endif

...

LOG(DEBUG,("user "<<getUserName()<<" hat sich eingeloggt."));
...
LOG(DEBUG,("foo("<<x<<','<<y<<")="<<foo(x,y));

[volkardus]

Maddes schrieb:

// am Anfang
int main() {
  Log::open( "name.log" );
  ...
}
// in einer x-beliebigen Funktion
Log::write( "dies wird geloggt" );

nö, eher nicht. es mag zwar ein wenig performance bvringen, das file nicht für jede zeile (bei mir sinds ja zeilen und nicht <<-aufrufe) zu öffnen und zu schließen, aber gemeinerweise sind die fehler, die das prog hart terminieren und dabei dann lustigerweise eine halbgeschriebene logdatei zurücklassen die gemeinsten. logs zu debuggingzwecken sollten dafür sorgen, daß wenn man vor einen kritischen aufruf eine logzeile sendet, die auch echt gespeichert ist, bevor der kritische aufruf ausgeführt wird.
das dauernde öffnen und schließen mag performance kosten. vielleicht reicht auch offenhalten und nach jeder zeile flushen. vielleicht auch nur fluschen in einem bestimmten loglevelbereich.
sollte trotzdem das logging wegen des flushens zu lahm sein, ist es natürlich angebracht, das flushen doch wegzumachen. oder, wenn man ein wenig zeit hat, loggt man einfach nicht in eine datei, sondern in eine pipe eines parallel gestarteten programms. das andere programm kann nicht vom sterbenden prog runtergezogen werden, weshalb es nie flushen muss. vielleicht startet der loggende prozess den pipe-to-file-prozess beim start und zwar so, daß der pipe-to-file-prozess nicht child ist, sondern am leben bleibt, wenn der loggende prozess stirbt. und der pipe-to-file-prozess überwacht den loggenden prozess und wenn der stirbt, flusht er und begeht selbstmord. überwachen? er begeht einfach selbstmord, wenn die pipe zusammenbricht. fühlt sich schon nach füchterlich guter performance an (aber meistens den aufwand nicht wert). andererseits, sowas muss kann man ja immer wieder benutzen.