Upgrade the Slf4j logging in SiteMap's

[GoogleCodeExporter]

The upgrade I suggest is as follows:
When I find a log statement which concatenates text in a log statement I want 
to use slf4j "Parameterized" way of doing it.
Here is an example:
Current code:
LOG.debug("XML url = " + xmlUrl);

Suggested Code:
LOG.debug("XML url = {}", xmlUrl);

The suggested code is the slf4j way of doing that sort of String concatenation 
logging.

The logic is that if the user's log level is INFO for example, there is no need 
to waste CPU in doing that concatenation.
Our current code does the String concatenation anyway (even when on INFO level 
or higher), while the slf4j way doesn't do the concatenation unless in DEBUG 
level.

In log4j they realized the problem so they came with a workaround which is 
using a condition as follows:
if (LOG.isDebugEnabled()){
 LOG.debug("XML url = " + xmlUrl);
}

it solves the problem, but now we have 3 lines instead of 1.
(Slf4j does that condition like log4j, but it does it internally.).

Which brings me to my second suggestion:
Removing the check for the log level.

The check is needed when using old logging libraries like log4j v1.X
But when using slf4j there is no need for it as I wrote above.

So I want to change the following code:
if (LOG.isDebugEnabled()){
    StringBuffer sb = new StringBuffer("  ");
    sb.append(i).append(". ").append(url);
    LOG.debug(sb.toString());
}

By removing the unnecessary IF condition.

So it will look like the following:
StringBuffer sb = new StringBuffer("  ");
sb.append(i).append(". ").append(url);
LOG.debug(sb.toString());

And actually if both suggestions will be accepted then the above code will look 
like:
Current code:
if (LOG.isDebugEnabled()){
    StringBuffer sb = new StringBuffer("  ");
    sb.append(i).append(". ").append(url);
    LOG.debug(sb.toString());
}

Suggested code:
LOG.debug("  {}. {}", i, url);

Original issue reported on code.google.com by avrah...@gmail.com on 9 Apr 2014 at 6:15

[GoogleCodeExporter]

I have upgraded just the parser for now - I want to be sure it is ok before 
upgrading other files.

Please review my changes and feel free to comment on anything.

Original comment by avrah...@gmail.com on 10 Apr 2014 at 8:55

Attachments:

• Upgraded_the_Slf4J_code___Used_parameterized_log_calls_instead_of_using_String_concatenati.patch

[GoogleCodeExporter]

This patch looks pretty sound to me. Compiles and all tests pass. You've also 
improved some Javadoc which is nice and added a small TODO regarding moving 
SiteMapParserTest out of main directory and into test.
Anyone else?
@Avi, can you please open an issue for addressing the TODO?
This is a trivial patch but an important one as we should not be pushing Test 
cases with runtime code.  
Finally, I've just changed the issue name to reflect the fact that this is 
_only_ addressing SiteMap's ATM. Feel free to open additional issues where you 
see fit Avi.
Thanks for your contribution :) 

Original comment by lewis.mc...@gmail.com on 12 Apr 2014 at 5:16

• Changed title: Upgrade the Slf4j logging in SiteMap's

[GoogleCodeExporter]

I will take care of the todo, no problem.

But I will address it in a few days, I think I understand why it is there and I 
might have a better solution, I will keep you posted.

Original comment by avrah...@gmail.com on 12 Apr 2014 at 8:12

[GoogleCodeExporter]

This is a better Patch.

It includes the changes of the previous patch (except for the Tester file which 
should be in a different issue anyway), but adds the changes to the rest of the 
sitemaps package files.

So please disregard the previous patch and use this in it's stead

Original comment by avrah...@gmail.com on 14 Apr 2014 at 6:27

Attachments:

• Issue34__Upgrade_Sitemap_slf4j_support,_with_some_JavaDoc_upgrades.patch

[GoogleCodeExporter]

Committed @revision 127 in trunk.
@Avi, I noticed that there is a block comment introduced in this patch which 
states that the method will be used in a previous patch. As long as we come 
back to this it is fine.
Thank you for your contributions and apologies for taking so long to get around 
to reviewing and committing.

Original comment by lewis.mc...@gmail.com on 29 May 2014 at 8:30

• Changed state: Fixed

[GoogleCodeExporter]

Original comment by avrah...@gmail.com on 12 Jul 2014 at 8:23