I am currently working on a new version of my application, and I just finished rebuilding the log part. It works well but I am skeptical about the choices I made. I am a student and I work alone in some side projects, and I am afraid of developing bad habits.
I use java.util.logging, because it is simple enough for what I need to do.
The logger will be used by several part of the application (but not at the same time). Each executable will have a logging file. All configurations are stored in Settings
class.
Main class :
public class MyLogger
{
public static Logger LOGGER = null; // Logger use by Spider, Cleaner, Checker
private static Handler logFile = null; // Logger file
/**
* Initialize logger before first use
*
* @param folderName sub-folder after the log folder
* @param fileName log file name (without extension)
*/
public static void initLogger(String folderName, String fileName)
{
if (MyLogger.LOGGER != null)
throw new IllegalStateException("Logger already instantiated");
MyLogger.LOGGER = Logger.getLogger(MyLogger.class.getName());
// Check if log is not disable
if (Settings.getProp().getLogLevel() != Level.OFF)
{
String folderPath = Settings.getProp().getLogPath() + folderName;
File dir = new File(folderPath);
// if the directory does not exist, create it (recursively)
if (!dir.exists())
{
try
{
dir.mkdirs();
}
catch (SecurityException e)
{
e.printStackTrace(); // Throw RunTime instead ?
}
}
try
{
// Open log file
MyLogger.logFile = new FileHandler(folderPath + File.separator + fileName + ".log", true);
MyLogger.logFile.setEncoding("UTF-8");
// Attache file to logger
MyLogger.LOGGER.addHandler(MyLogger.logFile);
}
catch (SecurityException | IOException e)
{
e.printStackTrace(); // Throw RunTime instead ?
}
}
LOGGER.setLevel(Settings.getProp().getLogLevel());
// Console Text formatter
LOGGER.setUseParentHandlers(false);
MyConsoleHandler handler = new MyConsoleHandler();
handler.setLevel(Settings.getProp().getConsoleLevel());
handler.setFormatter(new MyFormatter());
LOGGER.addHandler(handler);
LOGGER.config("Logger Started : " + folderName + " - " + fileName);
LOGGER.config(Settings.getProp().toString()); // Print application settings
}
}
Formatter :
public class MyFormatter extends Formatter
{
// Create a DateFormat to format the logger timestamp.
private static final DateFormat df = new SimpleDateFormat("H:mm:ss.SSS");
/* (non-Javadoc)
* @see java.util.logging.Formatter#format(java.util.logging.LogRecord)
*/
@Override
public String format(LogRecord record)
{
StringBuilder builder = new StringBuilder(1000);
builder.append(df.format(new Date(record.getMillis()))).append(" "); // time
builder.append("(").append(record.getThreadID()).append(") "); // Thread ID
builder.append("[").append(record.getLevel()).append("] "); // level
builder.append(formatMessage(record)); // message
builder.append("\n");
return builder.toString();
}
}
Console Handler :
public class MyConsoleHandler extends ConsoleHandler
{
/* (non-Javadoc)
* @see java.util.logging.Handler#publish(java.util.logging.LogRecord)
*/
@Override
public void publish(LogRecord record)
{
try
{
if(record.getLevel().intValue() >= this.getLevel().intValue())
{
String message = getFormatter().format(record);
if (record.getLevel().intValue() >= Level.WARNING.intValue())
{
System.err.write(message.getBytes());
}
else
{
System.out.write(message.getBytes());
}
}
} catch (Exception exception)
{
reportError(null, exception, ErrorManager.FORMAT_FAILURE);
return;
}
}
}
Example of use :
public static void main(String[] args)
{
MyLogger.initLogger("test", "001");
MyLogger.LOGGER.info("Testing message");
MyLogger.LOGGER.warning("A warning message !");
}
Example of output (console) :
22:43:26.918 (1) [CONFIG] Logger Started : test - 001
22:43:26.921 (1) [CONFIG] Appication Settings :
- [...]
- Console Level : CONFIG
- Log Level : FINER
- Log Path : ./log/
- [...]
- OutputQueueLimit : 1000 items
22:43:26.922 (1) [INFO] Testing message
22:43:26.922 (1) [WARNING] A warning message !
-
\$\begingroup\$ Welcome to Code Review! I hope you get some great answers. \$\endgroup\$Phrancis– Phrancis2016年08月16日 16:33:40 +00:00Commented Aug 16, 2016 at 16:33
1 Answer 1
1) Braces style
Putting opening curly braces on new lines is fine, though the big majority of Java developers don't do it and it wastes space.
Not putting curly braces around single line statements is dangerous and many people will discourage you from doing it!:
https://stackoverflow.com/questions/8020228/is-it-ok-if-i-omit-curly-braces-in-java
https://softwareengineering.stackexchange.com/questions/16528/single-statement-if-block-braces-or-no/16530
2) capitalized field names
Capitalized field names (LOGGER) are classically reserved for final constants, your LOGGER is not final. Meanwhile your DateFormat df should be capitalized.
3) Properly build paths
Use the File constructor to combine path parts:
https://stackoverflow.com/questions/412380/combine-paths-in-java
I guess making your ultimate File like File file = new File(folderName, fileName + ".log")
and then calling file.getParentFile().mkdirs();
would be the most pleasant invocation.
Pass the path of the file to your FileHandler constructor.
4) Throw RuntimeException on SecurityException
Your logger won't work. Something is wrong and your program shouldn't just silently ignore it! I'd prefer it to crash at that point: "Fix the logging or I won't work!"
5) SimpleDateFormat is not threadsafe!
Evil JRE developers want to secretly and unexpectedly mess up your date formatting:
https://stackoverflow.com/questions/6840803/simpledateformat-thread-safety
You should at least be aware of that if you plan to use multithreading.
6) Append cascade
Your append cascade is somewhat painful to read: It's hard for me to see exactly what you are combining.
I'd put every .append() on a new line.
7) Append cascade
Is there any advantage to calling write(message.getBytes());
instead of println/print (message)? I have never seen that before.
8) Seperate loggers for different classes, no direct access
Normally you create one logger for each class it's used in and put it in a static final field. That way it's easier to control logging and find out where the log entry comes from.
9) HH instead of H
You might want to use HH instead of H in your SimpleDateFormat, so your log entries are better aligned and more consistent.
-
\$\begingroup\$ Great answer, can you tell me more about the point 8 ? For each execution of my application I want only one log file. \$\endgroup\$Opsse– Opsse2016年08月18日 17:15:04 +00:00Commented Aug 18, 2016 at 17:15
-
\$\begingroup\$ Take a deeper look at Javas default Logging API and usage: vogella.com/tutorials/Logging/article.html You see that static final Logger instances are acquired via
Logger.getLogger(CurrentClass.class.getName());
. You should probably attach your custom handlers to the root logger. \$\endgroup\$ASA– ASA2016年08月19日 10:04:23 +00:00Commented Aug 19, 2016 at 10:04