Logging Review

[meister-g]

Hallo zusammen,

ich habe mich damit zwar noch nicht näher beschäftigt ich weiß, dass es zu Logging verschiedene APIs gibt, wollte jetzt aber mal auf die Schnelle eine Logfile auf die Festplatte bannen ohne, dass ich verschiedene Level festlege und externe Libs einbinden muss.

Spricht gegen die folgende Implementierung etwas?
close() wird aufgerufen, wenn das Programm ordnungsgemäß beendet wird.
Es funktioniert alles einwandfrei, da ich mir aber keine größeren Gedanken darüber gemacht habe wollte ich das ganze mal Reviewen lassen.

import java.io.FileWriter;
import java.io.IOException;
import java.util.Date;

public class Logger {

	private FileWriter writer;
	
	private static Logger instance;
	
	public static synchronized Logger getInstance() {
		if (instance == null)
			instance = new Logger();
		return instance;
	}
	
	private Logger() { 
		
		try {
			writer = new FileWriter(SettingsManager.getInstance().getTempDirectory() + "log.txt", true);
			log("------------------------------------------------");
			log("------------------------------------------------");
			log("Starte session: " + new Date(System.currentTimeMillis()));
			log("------------------------------------------------");
			log("------------------------------------------------");
		} catch (IOException e) {}
	}
	
	
	public void log(Object o) {
		log(o.toString());
	}
	
	public void log(String text, boolean newLine) {
		try { 
			writer.write(text + (newLine? "\r\n" : ""));
			if (newLine)
				writer.flush();
		} catch (IOException e) {}
	
	}
	
	public void log(String text) {
		log(text, true);
	}
	
	public void close() {
		try { 
			log("--------------------------");
			log("--------------------------");
			log("Programm beendet");
			writer.close();
		} catch (IOException e) {}
	}
}

 

[FArt]

Würde beir mir in einem Review aus mehreren Gründen durchfallen. Hier einige davon:
- Es gibt etwas besseres bei Java (ohne Einbinden "fremder" Libraries). Klöppel nichts selber, was es besser gibt.
- Schlucke NIE Exceptions
- Lazy Initialisation für den Singleton? Wozu?
- Ressourcen sicher im finally-Block schließen
- Logger sollten threadsafe sein
- ...

Der Grund warum man Sachen, die schon existieren nicht selber klöppeln sollte? Man macht zu schnell zu viele Fehler, kann bei weitem nicht so viel leisten wie nötig, ...

 

[maki]

Spricht gegen die folgende Implementierung etwas?

Ja, alles.

Das letzte was die Welt braucht ist noch ein Loggingframework mit Fehlern(Exceptions verschlucken, fehlende Synchronosierung, etc. pp.) & weniger Funktionen & gar keiner Doku.

log4j ist mit Abstand eines der besten Loggingframeworks, wenn du weniger brauchst nimm doch einfach das Java Util Logging, letzteres braucht keine externen Libs sondern kommt mit dem JRE, aber wenn du es ernst meinst nimmst du log4j, eventuell mit einem vorgeschalteter SLF4J.

 

[meister-g]

ok, da habe ich auf die schnelle echt einiges nicht bedacht.

was spricht an dieser stelle gegen ein singleton, wenn ich durch einen einfachen einzeiler loggen möchten?
bzw. wie löse ich das besser?

und was ich mich auch frage: wie unterdrücke ich bei jedem log die einzeilige meldung
Dec 1, 2010 11:59:28 AM package class
?

[JAVA=42]

import java.io.IOException;
import java.util.Date;
import java.util.logging.FileHandler;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.logging.SimpleFormatter;

public class Log {

private Logger logger;
private FileHandler fh;

private static Log instance;

public static synchronized Log getInstance() {
if (instance == null)
instance = new Log();
return instance;
}

public Log() {

logger = Logger.getLogger("MyLog");
try {
fh = new FileHandler(SettingsManager.getInstance().getTempDirectory() + "log.txt", true);
logger.addHandler(fh);
logger.setLevel(Level.ALL);
SimpleFormatter formatter = new SimpleFormatter();
fh.setFormatter(formatter);
} catch (SecurityException e) {
e.printStackTrace();
} catch (IOException e) {
e.printStackTrace();
}

log("------------------------------------------------");
log("------------------------------------------------");
log("Starte session: " + new Date(System.currentTimeMillis()));
log("------------------------------------------------");
log("------------------------------------------------");

}


public void log(Object o) {
log(o.toString());
}

public void log(String text) {
logger.log(Level.INFO, text);
}

public void close() {
log("--------------------------");
log("--------------------------");
log("Programm normal beendet");
}
}
[/code]

 

[FArt]

Du brauchst eine Logger Instanz nicht mehr zu wrappen... Benutze sie einfach. Ein Logger ist in der Regel ein Singleton, aber geeignet für konkurrierende Zugriffe aus mehreren Threads. Die Frage war ja, warum lazy-initialisiert.

Auch über Formatierung brauchst du dir so keine Gedanken machen.... Für eine sinnvolle Datumsausgabe nutze SimpleDateFormat...

Und noch mal der dringende Hinweis: benutze einfach eine vorhandene Logging-API... pur... sonst nix...

 

[meister-g]

ich würde halt gerne wrappen um nicht immer den namen des loggers und einen level mit angeben zu müssen.
aber ich sehe ein, dass es besser ist den logger der java se direkt zu benutzen.

wie ich die datumsanagbe umformatiere ist klar.
aber wie stelle ich die anzeige ab?
damit das log lesbar und auch nicht doppelt so lang ist würde wie im sourcecode gerne einmal den start der session loggen, danach interessiert mich der zeitpunkt aber nicht mehr.

 

[FArt]

In dem Fall loggt man eine Logmeldung bei Start der Session mit der Uhrzeit.

... wobei das sowieso Käse ist. Normalerweise gehören Uhrzeit (relativ oder absolut), Klasse, Loglevel, Thread, ... usw. nciht in die Logmeldung. Dafür konfiguriert man einen Appender mit dem entpsrechendne Pattern für den Formatter (jetzt mal für log4j gesprochen).