OOP



  • Hey Leute,
    Ich arbeite zurzeit an einem Logger jedoch habe ich ein Problem:
    Dieser Code wurde von mir schlampig geschrieben (meine meinung ;))
    un ich wüde gerne wissen wie ich ihn verbessern könnte:

    void Logger::send(const std::string& message)
    {
        time_t t = time(0);
        struct tm *now = localtime(& t);
        std::ofstream log("server.log", std::ios::app);
        write(localtime(& t), log, message);
        std::cout << now->tm_hour << ":" << now->tm_min << ":" << now->tm_sec << " " << message + "\n";
    }
    

  • Mod

    log sollte eine Klassenvariable sein.

    Soll eine der Ausgaben wirklich fix nach cout geschrieben werden?

    Warum rufst du 2x localtime auf, wenn du doch das Ergebnis vom ersten Mal noch hast?

    Können mehrere Threads auf den gleichen Logger zugreifen?



  • Hallo SeppJ,

    Du hast recht mir ist gar ncht aufgefallen das localtime 2x erstellt wird. Habe das jetzt geändert allerdings weiss ich nicht ob ich für den Logger printf verwenden soll daher mir das nicht empfohlen wurde. Könntest du mir bitte ein beispiel zeigen wie du es machen würdest? Hier der ganze aufbau des Loggers der von dem ganzen Programm aufgerufen wird:

    void Logger::info(const std::string& message)
    {
        send("[INFO] " + message);
    }
    
    void Logger::send(const std::string& message)
    {
        time_t t = time(0);
        struct tm *now = localtime(& t);
        std::ofstream log("server.log", std::ios::app);
        write(now, log, message);
        std::cout << now->tm_hour << ":" << now->tm_min << ":" << now->tm_sec << " " << message + "\n";
    }
    
    Logger & Logger::getInstance()
    {
        static Logger Instance;
        return Instance;
    }
    
    void LOG(const std::string& message)
    {
        Logger::getInstance().info(message);
    }
    


  • Mal ne ganz doofe Frage: was macht die "Logger" Klasse? Hat die auch irgendwelche Member?


  • Mod

    Ob du printf oder cout benutzt ist doch egal, das ändert nichts an dem, was ich gesagt habe. Beide schreiben nach stdout und ich stelle in Frage, ob das eine gute Idee ist.

    Die erste und wichtigste Bemerkung, über log als Klassenvariable, scheinst du komplett zu ignorieren.



  • aidenchloe7 schrieb:

    . o O ( Ich frag mich immer, wie solche Menschen reden *eeek* )



  • Hey Sepp warum sollte LOG eine Klassenvariable sein?



  • aidenchloe7 schrieb:

    Hey Sepp warum sollte LOG eine Klassenvariable sein?

    Du oeffnest die Datei jedes Mal und schliesst sie wieder. Das ist sehr langsam und kann zu Fehlern fuehren, wenn sich die Datei irgendwann mal nicht oeffnen laesst.
    Stattdessen lohnt es sich, sie einmal zu oeffnen und solange offen zu halten, wie man braucht.



  • Aber Marthog ich habe eine Logger instance erstellt. Weshalb sollte ich noch eine weitere erstellen. Wenn ich etwas Logger verwende ich:

    LOG("Das ist mien Beispiel Log");
    


  • Trotzdem öffnest du die Datei durch den Erstellen des ofstreams jedes Mal!

    BTW: ><((((°> ?



  • Du hast eine Loggerinstanz, aber oeffnest die Datei immer wieder neu in

    std::ofstream log("server.log", std::ios::app);
        write(now, log, message);
        std::cout << now->tm_hour << ":" << now->tm_min << ":" << now->tm_sec << " " << message + "\n";
    

    Oeffne die Datei einmal in Constructor und behalte sie dann offen.



  • So un besser 😃

    void Logger::send(const std::string& message)
    {
        time_t t = time(0);
        struct tm *now = localtime(& t);
        static std::ofstream log("server.log", std::ios::app);
        write(now, log, message);
        std::cout << now->tm_hour << ":" << now->tm_min << ":" << now->tm_sec << " " << message + "\n";
    }
    


  • Wieso dann überhaupt eine Klasse? Schreibe direkt eine freie Funktion ohne die Klasse.



  • Nathan ich verstehe das nicht, was für eine Klasse/Funktion?



  • Du hast eine Klasse Logger ohne Membervariablen als Singleton.
    Sie ist absolut überflüssig. Schreibe direkt eine freie Funktion.



  • Mit freire Funktionen meinst du schon externe?


Log in to reply