Technically, I think that is SLF4J syntax (but maybe Log4J2 supports this
format now as well; anyway).
Still, you should be careful with log statements like...
logger.debug("Logging in user {}" + user);
Assuming the User class itself and an "informative" and properly
constructed toString() method. So use it judiciously and wisely.
On Fri, Sep 7, 2018 at 11:18 AM, Dan Smith <dsm...@pivotal.io> wrote:
> I think this pattern is a holdover from using log4j 1. In that version, you
> added an if check to avoid unnecessary string concatenation if debug was
> enabled.
>
>
> 1. if (logger.isDebugEnabled()) {
> 2. logger.debug("Logging in user " + user.getName() + " with
> birthday " + user.getBirthdayCalendar());
> 3. }
>
>
> Log4j2 lets you pass a format string, so you can just do this:
>
> logger.debug("Logging in user {} with birthday {}", user.getName(),
> user.getBirthdayCalendar());
>
>
> These examples come from the log4j2 docs:
>
> https://logging.apache.org/log4j/2.0/manual/api.html
>
> -Dan
>
> On Fri, Sep 7, 2018 at 10:50 AM, Galen O'Sullivan <gosulli...@apache.org>
> wrote:
>
> > Hi all,
> >
> > I've noticed a pattern in Geode where we wrap a log call in a check at
> the
> > same level, such as:
> >
> > if (logger.isDebugEnabled()) {
> > logger.debug("cleaning up incompletely started
> > DistributionManager due to exception", r);
> > }
> >
> > Is there any reason to do this? To my mind, it's an extra conditional
> that
> > should essentially be the first check inside `logger.debug(...)` anyways,
> > and it complicates the code and makes it less readable. I've even seen
> > places in the code which have `if (logger.isDebugEnabled())
> > logger.trace(...))` and such.
> >
> > I would like to propose that unless there is a compelling reason to use
> > this pattern, we remove all extra checks entirely.
> >
> > Galen
> >
>
--
-John
john.blum10101 (skype)
