se-edu / addressbook-level3

:ab::three: Address Book sample application (Level 3)
https://se-education.org/addressbook-level3
MIT License
27 stars 411 forks source link

Null class logger not logging to logger output #173

Closed Eclipse-Dominator closed 1 year ago

Eclipse-Dominator commented 1 year ago

When a logger is created with a null class, it's logs are not being logged into the log file when all logs should be recorded in the log file.

If we create a logger using a null class for instance

Logger logger = LogsCenter.getLogger(Object.class.getSuperclass());
// Object.class.getSuperclass() gives a null class value
// since getLogger is overload with string type, this prevent ambiguity when compiling.

Logs made via this logger will not be able to write logs to the addressbook.log.0 file.

This is due to an incorrect function call to LogsCenter.getLogger(str) which enables logs to be written to a log file as well as other pipelines.

    /**
     * Creates a logger with the given name.
     */
    public static Logger getLogger(String name) {
        Logger logger = Logger.getLogger(name);
        logger.setUseParentHandlers(false);

        removeHandlers(logger);
        addConsoleHandler(logger);
        addFileHandler(logger);

        return Logger.getLogger(name);
    }

However, when a null class is being created,

    /**
     * Creates a Logger for the given class name.
     */
    public static <T> Logger getLogger(Class<T> clazz) {
        if (clazz == null) {
            System.out.println(clazz);
            return Logger.getLogger("");
        }
        return getLogger(clazz.getSimpleName());
    }

We can see that the default Logger.getLogger() is called instead which prevents the handlers from being added.


We can verify this by modifying a logger.

    public static final Version VERSION = new Version(0, 2, 0, true);

    private static final Logger logger = LogsCenter.getLogger(Object.class.getSuperclass()); // from MainApp.class

    protected Ui ui;
    protected Logic logic;
    protected Storage storage;
    protected Model model;
    protected Config config;

    @Override

image We can see that logs of null class are not being properly logged.


As of now, no null loggers have been created but I think this should still be considered as a bug? or that the argument that null loggers logs should not be logged in the log file.

damithc commented 1 year ago

If we don't want loggers to be added without a class, we can use an assertion instead. Perhaps that's simpler (especially because we currently don't have a need for such a logger -- as per YAGNI principle)?

Eclipse-Dominator commented 1 year ago

I think this will be for the best since there is no need to such a logger.