Java logger (of doom!), updated

Question

In a previous question, Java logger (of doom!), I had posted my own logger, written in Java. I have since taken the answers from that question and a few of my own things to update it.

New code:

/*
 * Syml, a free and open source programming language.
 * Copyright (C) 2021 William Nelson
 * mailto: catboardbeta AT gmail DOT com
 * Syml is free software, licensed under MIT license. See LICENSE
 * for more information.
 *
 * Syml is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
 */

package personal.williamnelson;

import java.io.Closeable;
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.OutputStreamWriter;
import java.nio.charset.StandardCharsets;
import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter;

public class Logger implements Closeable {

    public File logfile = new File("");

    protected FileOutputStream fos;
    protected OutputStreamWriter osw;

    protected DateTimeFormatter dtf = DateTimeFormatter.ofPattern("yyyy/MM/dd HH:mm:ss"); // example: 2021/27/5 12:07:23

    public void init(File fileToLog) {
        try {
            logfile = fileToLog;

            if (fileToLog.exists()) {
                clearFile(fileToLog);
            } else {
                if(!fileToLog.createNewFile()) {
                    System.out.println("An unrecognized error occurred while creating the logfile.");
                }
            }

        } catch (IOException e) { //NOSONAR
            System.out.println("An error occurred while creating a logfile. Are you sure that '" +
                                logfile.toString() + 
                               "' is a valid filename?"); 
            System.exit(1);
        }
        
        try {
            fos = new FileOutputStream(fileToLog, true);
            osw = new OutputStreamWriter(fos, StandardCharsets.UTF_8);
            clearFile(fileToLog);
        } catch (IOException e) {
            System.out.println("An unrecognized error occurred while start logging stream and " +
                               "writing to log stream, despite passing all tests.");
        }

    }

    public void log(Object o, int logType) throws IOException {
        // logType should be 0 = INFO, 1 = WARN, 2 = ERROR, 3 = FATAL
        osw.write(getTime() + ": ");
        printErrorType(logType);
        osw.write(o.toString() + '\n');
    }

    public void log(int i, int logType) throws IOException {
        osw.write(getTime() + ": ");
        printErrorType(logType);
        osw.write(String.valueOf(i) + '\n');
    }

    private String getTime() {
        LocalDateTime ldtNow = LocalDateTime.now();
        return ldtNow.format(dtf);
    }

    private void clearFile(File file) throws IOException {
        FileOutputStream fosClearer = new FileOutputStream(file, false);
        OutputStreamWriter oswClearer = new OutputStreamWriter(fosClearer);
        oswClearer.write("");
        oswClearer.close();
    }

    private void printErrorType(int errorType) throws IOException {
        switch (errorType) {
            case 0:
                osw.write("INFO: ");
                break;
            case 1:
                osw.write("WARN: ");
                break;
            case 2:
                osw.write("ERROR: ");
                break;
            case 3:
                osw.write("FATAL: ");
                break;
            default:
                throw new IllegalArgumentException("Parameter 'logType  " +
                        "of 'personal.williamnelson.Logger.log'" +
                        " must be one of the following: \n" +
                        "    0: INFO\n" +
                        "    1: WARN\n" +
                        "    2: ERROR\n" +
                        "    3: FATAL\n");
        }
    }

    public void close() throws IOException {
        osw.close();
    }
}
```

Miguel Alorda · Accepted Answer · 2021-06-02 17:30:11Z

3

Why use an int to indicate the log type when you can just use an enum That way, you could just write the enum to the OutputStreamWriter directly (while avoiding the worry of receiving an incorrect logType: IllegalArgumentException):

public class Logger implements Closeable {
  public enum LogType {
    FATAL, ERROR, WARN, INFO, DEBUG, TRACE;
  }
  ...
  public void log(Object o, LogType logType) throws IOException {
    osw.write(getTime() + ": " + logType + ": " + o.toString() + "\n");
  }
}

Also, you could just rewrite the log(int, LogType) method to reuse log(Object, LogType), to avoid duplicated code:

public void log(int i, LogType logType) throws IOException {
  log(Integer.valueOf(i), logType);
}

Regarding code style, there is no need to write an if-else-if this way:

if (foo) {
  doSomething();
} else {
  if (bar) {
    doSomethingElse();
  }
}

Unless you wished to do something in the else block, but outside the inner if, it is more readable to write it as such:

if (foo) {
  doSomething();
} else if (bar) {
  doSomethingElse();
}

edited Jun 2, 2021 at 17:30

answered Jun 2, 2021 at 17:22

Miguel Alorda

1,4237 silver badges14 bronze badges

\$\begingroup\$ I never would have even thought to use an enum, and I totally forgot about DEBUG and TRACE. Also, where did I use a strange if statement like that? \$\endgroup\$

CATboardBETA
– CATboardBETA

2021-06-02 17:36:22 +00:00
Commented Jun 2, 2021 at 17:36
\$\begingroup\$ @catboardbeta In you init method \$\endgroup\$

Miguel Alorda
– Miguel Alorda

2021-06-03 18:12:38 +00:00
Commented Jun 3, 2021 at 18:12

Add a comment |

Ayush Garg · Accepted Answer · 2021-06-02 20:32:12Z

Looking at your init() function and your try/catch blocks, shouldn't it be better to use System.err.println instead of System.out.println when you catch an IOException?

On the same lines, if this is supposed to be a generic class, shouldn't the consumer be doing the error handling? You should just hand them the IOException (add a throws IOException to your init() function and remove your try/catch blocks) and then force the consumer to handle it instead. This way the consumer has more control on what happens if something wrong happens.

Also, what is that extra log(int i, int logType) doing there? Shouldn't int get boxed into log(Object o, int logType)?

On the topic of that, you should definitely use enums (as @m-alorda said). It makes your code much easier to read and use.

Sara J · Accepted Answer · 2021-06-02 21:03:13Z

Is there a reason logfile can't be an argument of the constructor? The way I see it, a Logger that hasn't had init() called is just guaranteed to be in an invalid state and entirely useless until inited, so calling init in the constructor just seems like a good way to guarantee the Logger is in a useful state. Am I missing something?
I can imagine a situation where some application may want to run part of its code even though the logging isn't working, but since init calls System.exit() that doesn't seem to even be an option. Unless there's some particular reason init (or the Logger constructor) must never be allowed to throw exceptions, it should probably do that, rather than System.exit()
It feels a bit weird that clearFile can clear files other than the designated logfile. Especially since logfile is the only file that's ever passed to it, not that a logger should ever want to work with any other files anyway. Wouldn't it make more sense to just have it operate on the log file directly since a logger shouldn't really need to touch any other files anyway?

Stack Exchange Network

Java logger (of doom!), updated

3 Answers 3

You must log in to answer this question.

Linked

Hot Network Questions

Java logger (of doom!), updated

3 Answers 3

You must log in to answer this question.

Linked

Related

Hot Network Questions