Introduced SLF4J for logging (w/ Logback in tests)

[connollyst]

Fixes #150, #151, #152, #154, #155 and lays foundation for related issues.
Note: there is no use of verbosity in the GeospatialCoordinateEncoder and so #153 can be closed.

[connollyst]

Hi @rhyolight, can you add me to the "signed CL" list when you get a chance? Cheers

[coveralls]

Coverage increased (+0.32%) to 66.62% when pulling 2762c34 on connollyst:logging into 7d229c4 on numenta:master.

[coveralls]

Coverage increased (+0.63%) to 66.93% when pulling 7ea3bbf on connollyst:logging into 7d229c4 on numenta:master.

[connollyst]

@cogmission sorry about all the extra modified lines: even with auto-formatting disabled, IntelliJ strips tabs from blank lines. If you really want them, please do send over the Eclipse formatting standards you want to enforce.

[coveralls]

Coverage increased (+0.63%) to 66.93% when pulling 6d62c74 on connollyst:logging into 7d229c4 on numenta:master.

[connollyst]

@cogmission & @solovyevk, I used:
verbosity == 0 as INFO
verbosity == 1 as DEBUG
verbosity == 2 as TRACE

However, the use of TRACE is really rare in normal applications. I'd think we really want DEBUG in most cases. To avoid issues with this pull request I just ported it as-is.

[cogmission]

@connollyst Wow! You really went to work! Thanks!

I do have a question. In the following scenario, is it accepted practice to make the following method calls without testing for the current logging-level setting? I'm concerned about slowing down the code for times when we want to favor performance over output? In other words, shouldn't places like this be surrounded by some kind of if(Logger.level == TRACE) or some other similar semantic? (Please see the highlighted line here for an example: https://github.com/numenta/htm.java/pull/174/files#diff-f64f4121d6cbfa809d8a3ae0ad816df3R245)

[cogmission]

@connollyst
Can you click on the "Details" link next to the "Contributor Validator" failure above and fill out the licensing?

EDIT: I sent @rhyolight a message, he should see this and add you. I didn't see your message above that you had already signed the license agreement (above).

Merges brought in before your code also caused a failure in that code (fortunately only in two places visible from the "Details" link next to the Travis CI failure - in that person's submitted code "AdaptiveScalarEncoder".) Should be an easy fix.

[rhyolight]

@connollyst Maybe something went wrong, but I have not received your signed contributor license agreement. Have you filled out the online form at http://numenta.org/licenses/cl/ ?

[connollyst]

Yup, I've signed the agreement. I just forwarded you the confirmation email I had received.

[rhyolight]

@connollyst Thanks, got it, and done!

[cogmission]

@connollyst How do you like the labels? :-P (Of course I'm just joking with you)

[coveralls]

Coverage increased (+0.58%) to 68.61% when pulling 8be8976 on connollyst:logging into 722fdb2 on numenta:master.

[connollyst]

Hey @cogmission,

Should be all set now.

Cheers,
Sean

[cogmission]

@connollyst Did you see my question about surrounding Log calls with a test to make sure the log level warrants executing them? Otherwise we'll hit every single log statement that was added (a test in much the same way that if(verbosity == 2) <-- used to do?

[cogmission]

@connollyst Thanks for your hard work! I've since read that call sites involving String concatenations should have guards placed around them. So I will go through and fix these if any exist.

[connollyst]

Hey David,

I did, sorry I didn't respond sooner (I'm up in Switzerland skiing - off the grid for the most part).

So there are three ways to aspects to consider: readability, utility, and efficiency; not mutually exclusive of course.

In terms of readability, I think using a logger is as good as, if not better than using verbosity. More importantly, it's standard so the code is more intuitive to a (new) developer.

With respect to utility, dito. It's on par with verbosity and is industry standard.

Now, for efficiency. It's definitely not a silly question, especially considering writing to console/file is one of the most "tedious" things a program can do (grabbing locks, interacting with disk, etc.) I think there are two things to consider within this issue: computing the statement, & logging the statement.

The latter first, LOGGER.debug("hi") is more efficient than if(LOGGER.isDebugEnabled()){LOGGER.debug{"hi"}}, but using isDebugEnabled becomes more efficient as you add more debug statements in each clause. In reality, we're talking about, say, 1-5 debug statements at a time, and the "efficiency" difference is a boolean check. So it's negligible.

The real concern is in computing the statement itself. For comparison, LOGGER.debug(doCompute()) is less efficient than if(LOGGER.isDebugEnabled()){LOGGER.debug(doCompute())} when debugging is disabled. This is something to be aware of. As a general rule, you're not usually executing computation just for logging purposes, so this would be a code smell as is.

I saw some places where verbosity wrapped a loop or use of a StringBuffer, the former would be a case to use LOGGER.isDebugEnabled() and the latter could be, but is probably better replaced by a String literal (probably replaced by the compiler anyway).

More info here:
http://stackoverflow.com/questions/8444266/even-with-slf4j-should-you-guard-your-logging

My 2¢:
Don't wrap your log statements, just log intelligently. If you need to log some computation or loop & log, then you can wrap, but I'd think twice about what you're really trying to do.