HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaModerate

Custom Logging Class: Efficiency

Submitted by: @import:stackexchange-codereview··
0
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:

  • 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

  • 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 SimpleDateFormat objects and store it in a class level field instead of creating them each time.



  • also create your messages array only once as a field.



Robustness

  • use an enum instead of string for your warning levels



  • you are never closing your propFile stream.



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 as shortDateFormat, format as extendedDateFormat, dateApp as logFileName, appendValue as appendToFile, logLevel as maximalLogLevel (should this maybe be minimal?), logType as logLevel`.



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.