Grrr, meant...
logger.debug("Logging in user {}", user);
On Fri, Sep 7, 2018 at 11:37 AM, John Blum <jb...@pivotal.io> wrote:
> 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)
>
--
-John
john.blum10101 (skype)
