search

EXIficient / exificient-grammars

Java Implementation of EXI (grammars part)
http://exificient.github.io/java/
MIT License
4 stars 9 forks source link

Migration to SLF4J #22

Closed BBE78 closed 1 year ago

BBE78 commented 1 year ago

Use of SLF4J logger instead of System.(out|err).println()

BBE78 commented 1 year ago

I ask you to remove all if (LOGGER.isDebugEnabled()) wrappers since it is not necessary anymore with the logging framework in place...

Hi @danielpeintner, checking the log level before logging is a "best practice" to avoid memory allocation, string concatenation, for nothing... This rule is raised by several tools such as SonarQube https://rules.sonarsource.com/java/RSPEC-2629 I'm used to apply this recommendation in my source code, do you still want the check to be removed?

danielpeintner commented 1 year ago

I don't have a very strong opinion but I guess we both can agree that the code looks nicer and less messy without this is additional if (LOGGER.isDebugEnabled()) { wrapper...

Moreover, with the formatting style of LOGGER.debug("Foo '{}' is happening", "x"); the string is actually not concatenated.

It would be different if we do something like the following LOGGER.debug("Foo " + "xyz" + " is happening);

see also some information here: https://logging.apache.org/log4j/log4j-2.3.2/performance.html

Anyhow, as I said, IF YOU STRONGLY PREFER to keep it as is I will not argue strongly... :)

BBE78 commented 1 year ago

All debug level checks removed