Was ist das beste Design in dieser Klasse?

Question

nehmen diese Klasse:

public class Logger 
{ 
    static TextWriter fs = null; 

    public Logger(string path) 
    { 
     fs = File.CreateText(path); 
    } 

    public static void Log(Exception ex) 
    { 
     ///do logging 
    } 

    public static void Log(string text) 
    { 
     ///do logging 
    } 
} 

und ich habe dies wie zu verwenden:

Logger log = new Logger(path); 

und dann Logger.Log() verwenden, um sich, was ich will. Ich benutze nur einen Logger. ist die Frage: Ist das ein gutes Design? eine Klasse instanziieren und dann immer ihre statische Methode nennen? jeder Vorschlag Ertrag in besserem Design wird geschätzt.

bearbeitet basierend auf Marc ‚s Antwort:

ich in der letzten Zeile von Log spülen und es gibt keine Notwendigkeit für mich, um die Datei zu lesen, während es geöffnet ist, das Problem mit der Datei nicht sauber geschlossen ist richtig. Diese Klasse erfüllt einfach meine Anforderungen und es ist nicht notwendig, dafür threadsicher zu sein. Ich möchte nur den Instanziierungsteil lesen, ich sollte in den SetPath kommen, den du gesagt hast, irgendeinen Vorschlag zum Schließen der Datei?

Answer 1

Ja, einen Konstruktor nur dafür zu haben ist schlechtes Design. Eine statische Methode, die nur einmal aufgerufen werden kann (sonst löst eine Ausnahme) würde besser scheinen. Sie würden den Pfad während App-Start, etc.

Dann können Sie entweder ein static class, oder ein Singleton, wenn es erforderlich ist, einige Schnittstelle-basierte Szenario zu erfüllen.

Weiter: Sie müssen müssen Synchronisierung hier hinzufügen! Das ist nicht threadsicher. Wenn zwei Threads gleichzeitig versuchen, zu loggen, würde ich erwarten, dass dies fürchterlich zusammenbricht. Es muss nicht komplex sein; am einfachsten:

private readonly object syncLock = new object(); 
public static void Log(string value) { 
    lock(syncLock) { 
     //... 
    } 
} 

(aber beachten Sie, dass dies einige blockierende Kosten entstehen können, die mit anspruchsvollere Codes verbessert werden kann - siehe unten)

Es gibt vorhandenen Logging-Bibliotheken, die mehr Fragen der viel denken - Dateipartitionierung, async (um zu verhindern, dass Ihr Code von IO blockiert wird), Batching, etc .; warum nicht einfach einen von ihnen benutzen? Insbesondere wird Ihre Datei im Moment nicht sauber beim Beenden der App geschlossen, spült nicht regelmäßig und hält die Datei die meiste Zeit gesperrt. Nicht gut.

Answer 2

Nein, das macht keinen Sinn. Jedes Mal, wenn ein Logger instanziiert wird, wird der statische TextWriter überschrieben, was sich auf alle Konsumenten der Klasse auswirkt. Wenn Sie den Instanzkonstruktor behalten möchten, sollten Sie den TextWriter zu einem Instanzfeld machen und die Methoden sollten Instanzmethoden sein.

Alternativ können Sie log4net verwenden, das diese Art der Protokollierung für Sie übernimmt.

Answer 3

Ich denke, Sie sollten ganze Klasse statisch mit statischen Eigenschaften machen, so dass Sie den Protokollpfad einrichten können.

public static class Logger 
{ 
    static TextWriter fs = null; 

    public static string FileName 
    { 
     set 
     { 
     fs = File.CreateText(value); 
     } 
    } 

    public static void Log(Exception ex) 
    { 
     if(fs == null) return; 
     ///do logging 
    } 

    public static void Log(string text) 
    { 
     if(fs == null) return; 
     ///do logging 
    } 
}