patternjavaModerate
Custom Logging Class: Efficiency
Viewed 0 times
customefficiencyloggingclass
Problem
I wrote a logging class for several of my applications, but I have noticed that it is fairly in-efficient. Can anyone suggest improvements?
What I am looking for:
Please be as nit-picky as possible, I'd like to make this class as robust as possible.
```
import java.io.FileInputStream;
import java.io.FileWriter;
import java.io.IOException;
import java.io.PrintWriter;
import java.text.SimpleDateFormat;
import java.util.Date;
import java.util.Properties;
class FunctionLogging
{
private String path;
private boolean appendToFile = false;
public FunctionLogging(String file_path, int logLevel, boolean append_value)
{
path = file_path; //used as a placeholder variable for the output path. Can be hard-coded but is preferredly configurable from properties file
appendToFile = append_value; //Set as true when called in each class used in. true appends line to the end of the file, false overwrites file contents
}
public void writeToFile(int logType, String textLine) throws IOException
{
FileInputStream propFile = new FileInputStream("config.ini");
Properties config = new Properties(System.getProperties());
config.load(propFile);
String tempLev = config.getProperty("loggingLevel");
int logLevel = Integer.parseInt(tempLev);
if(logType<=logLevel)
{
//This section is devoted to creating a timestamp of the instance when the output string is created.
Date date = new Date();
SimpleDateFormat format = new SimpleDateFormat();
format = new SimpleDateFormat("dd-MM-yyyy|HH:mm:ss:SSSSSS|");
String timeStamp
What I am looking for:
- Pointers on improving efficiency
- Tips for lowering the code-footprint
- Suggested functional improvements
Please be as nit-picky as possible, I'd like to make this class as robust as possible.
```
import java.io.FileInputStream;
import java.io.FileWriter;
import java.io.IOException;
import java.io.PrintWriter;
import java.text.SimpleDateFormat;
import java.util.Date;
import java.util.Properties;
class FunctionLogging
{
private String path;
private boolean appendToFile = false;
public FunctionLogging(String file_path, int logLevel, boolean append_value)
{
path = file_path; //used as a placeholder variable for the output path. Can be hard-coded but is preferredly configurable from properties file
appendToFile = append_value; //Set as true when called in each class used in. true appends line to the end of the file, false overwrites file contents
}
public void writeToFile(int logType, String textLine) throws IOException
{
FileInputStream propFile = new FileInputStream("config.ini");
Properties config = new Properties(System.getProperties());
config.load(propFile);
String tempLev = config.getProperty("loggingLevel");
int logLevel = Integer.parseInt(tempLev);
if(logType<=logLevel)
{
//This section is devoted to creating a timestamp of the instance when the output string is created.
Date date = new Date();
SimpleDateFormat format = new SimpleDateFormat();
format = new SimpleDateFormat("dd-MM-yyyy|HH:mm:ss:SSSSSS|");
String timeStamp
Solution
Efficiency
Robustness
Readability
There are probably more improvements, but with this your code already looks a bit nicer:
Or with enums:
- the biggest improvement will be to not open the config file each time you log something, but only once when instantiating the logger.
- you could also open the log file only once, but it might not be a good idea to have it open for the whole lifetime of the program.
- create only one instance of the two
SimpleDateFormatobjects and store it in a class level field instead of creating them each time.
- also create your
messagesarray only once as a field.
Robustness
- use an enum instead of string for your warning levels
- you are never closing your
propFilestream.
Readability
- follow the standard Java indent style: curly brackets go on the same line as the opening statement.
- use more spaces (around
- don't do this: timeStamp+id+++"|"
.
- use camelCase for all variable names, don't mix it with snake_case except for final static fields.
- use JavaDoc style comments.
- remove variables that are only used in one place.
- use try with resources to save the close statement.
- choose better variable names. app
would be clearer asshortDateFormat,formatasextendedDateFormat,dateAppaslogFileName,appendValueasappendToFile,logLevelasmaximalLogLevel(should this maybe be minimal?),logTypeaslogLevel`.
There are probably more improvements, but with this your code already looks a bit nicer:
public class FunctionLogging {
private String path;
private boolean appendToFile = false;
private int logLevel;
private String[] messages = new String[] {"FATAL", "WARNING", "NOTICE", "DEBUG"};
private SimpleDateFormat format = new SimpleDateFormat("dd-MM-yyyy|HH:mm:ss:SSSSSS|");
private SimpleDateFormat app = new SimpleDateFormat("dd-MM-yyyy");
/**
*
* @param filePath used as a placeholder variable for the output path. Can be hard-coded but is preferredly configurable from properties file
* @param logLevel set as true when called in each class used in. true appends line to the end of the file, false overwrites file contents
* @param append_value TODO
*/
public FunctionLogging(String filePath, int logLevel, boolean appendValue) throws IOException {
path = filePath;
appendToFile = appendValue;
loadConfig();
}
private void loadConfig() throws IOException {
try (FileInputStream propFile = new FileInputStream("config.ini")) {
Properties config = new Properties(System.getProperties());
config.load(propFile);
logLevel = Integer.parseInt(config.getProperty("loggingLevel"));
}
}
public void writeToFile(int logType, String textLine) throws IOException {
if (logType <= logLevel) {
Date date = new Date();
String timeStamp = format.format(date);
String dateApp = app.format(date);
FileWriter logWriter = new FileWriter(path + dateApp + ".log", appendToFile);
try (PrintWriter loggerLines = new PrintWriter(logWriter)) {
loggerLines.println(timeStamp + "1|" + messages[logType] + "|" + textLine);
}
}
}
}Or with enums:
public class FunctionLogging {
public enum Level {
FATAL("FATAL", 1),
WARNING("WARNING", 2),
NOTICE("NOTICE", 3),
DEBUG("DEBUG", 4);
private final String message;
private final int level;
private Level(String message, int level) {
this.message = message;
this.level = level;
}
@Override
public String toString() {
return message;
}
public int getLevel() {
return level;
}
}
// [...]
public void writeToFile(Level level, String textLine) throws IOException {
if (level.getLevel() <= logLevel) {
// [...]
try (PrintWriter loggerLines = new PrintWriter(logWriter)) {
loggerLines.println(timeStamp + "1|" + level.toString() + "|" + textLine);
}
}
}
// [...]
// usage:
writeToFile(FunctionLogging.Level.DEBUG, "something happened");Code Snippets
public class FunctionLogging {
private String path;
private boolean appendToFile = false;
private int logLevel;
private String[] messages = new String[] {"FATAL", "WARNING", "NOTICE", "DEBUG"};
private SimpleDateFormat format = new SimpleDateFormat("dd-MM-yyyy|HH:mm:ss:SSSSSS|");
private SimpleDateFormat app = new SimpleDateFormat("dd-MM-yyyy");
/**
*
* @param filePath used as a placeholder variable for the output path. Can be hard-coded but is preferredly configurable from properties file
* @param logLevel set as true when called in each class used in. true appends line to the end of the file, false overwrites file contents
* @param append_value TODO
*/
public FunctionLogging(String filePath, int logLevel, boolean appendValue) throws IOException {
path = filePath;
appendToFile = appendValue;
loadConfig();
}
private void loadConfig() throws IOException {
try (FileInputStream propFile = new FileInputStream("config.ini")) {
Properties config = new Properties(System.getProperties());
config.load(propFile);
logLevel = Integer.parseInt(config.getProperty("loggingLevel"));
}
}
public void writeToFile(int logType, String textLine) throws IOException {
if (logType <= logLevel) {
Date date = new Date();
String timeStamp = format.format(date);
String dateApp = app.format(date);
FileWriter logWriter = new FileWriter(path + dateApp + ".log", appendToFile);
try (PrintWriter loggerLines = new PrintWriter(logWriter)) {
loggerLines.println(timeStamp + "1|" + messages[logType] + "|" + textLine);
}
}
}
}public class FunctionLogging {
public enum Level {
FATAL("FATAL", 1),
WARNING("WARNING", 2),
NOTICE("NOTICE", 3),
DEBUG("DEBUG", 4);
private final String message;
private final int level;
private Level(String message, int level) {
this.message = message;
this.level = level;
}
@Override
public String toString() {
return message;
}
public int getLevel() {
return level;
}
}
// [...]
public void writeToFile(Level level, String textLine) throws IOException {
if (level.getLevel() <= logLevel) {
// [...]
try (PrintWriter loggerLines = new PrintWriter(logWriter)) {
loggerLines.println(timeStamp + "1|" + level.toString() + "|" + textLine);
}
}
}
// [...]
// usage:
writeToFile(FunctionLogging.Level.DEBUG, "something happened");Context
StackExchange Code Review Q#84500, answer score: 12
Revisions (0)
No revisions yet.