Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ENH] Enhancement regarding logging #282

Open
hillgert opened this issue Oct 29, 2021 · 1 comment
Open

[ENH] Enhancement regarding logging #282

hillgert opened this issue Oct 29, 2021 · 1 comment
Labels
maintenance Dependency management and legacy code cleanups

Comments

@hillgert
Copy link
Collaborator

Description
Within the Java code there are a log of place where debug and info logging is done without checking if according log level is activated or not.

It should also be considered whether logging the information is necessary or useful at all in the corresponding situations.

Also the code contains a huge amount of comment lines like the following. These useless comments blowup up the code and should also be avoided.

// System.out.println(...)

Example

if (DATAINFO.getProperty("userAccountNotification") != null)
    DATAINFO.setProperty("user_account_notification", DATAINFO.getProperty("userAccountNotification"));
logger.debug("DataInfo..." + DATAINFO);

Solution

if (DATAINFO.getProperty("userAccountNotification") != null)
    DATAINFO.setProperty("user_account_notification", DATAINFO.getProperty("userAccountNotification"));
if (logger.isDebugEnabled()) {
    logger.debug("DataInfo..." + DATAINFO);
}

Benefits

  1. Creating new unnessary Java Object (String is also an object) - especially within loops - will be avoided as this is not needed to be done when according log level is disabled.
  2. Probably the code would perform a little better in some situtations as according logging functionality will not be performed.
  3. Removing unnecessary comment lines makes code more compact and therefore a little better to read.
@toskrip
Copy link
Collaborator

toskrip commented Nov 4, 2021

It depends. If the debug message is concated it may bring some improvement in performance (I don't think we will notice it). However it is a lot of extra code and some may say it is less readable. This check (whether specific log level is enabled) is done internally in a logging library anyway so I would not recommend to bring it to application level. The solution for improving performance is to use formatted debug method instead of concating String:

debug(String format, Object arg)

@toskrip toskrip added the maintenance Dependency management and legacy code cleanups label Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Dependency management and legacy code cleanups
Projects
None yet
Development

No branches or pull requests

2 participants