Check the recommended patterns for java8 [1], I think the lambda syntax makes the conditional less needed.
Anthony [1] https://logging.apache.org/log4j/2.0/manual/api.html <https://logging.apache.org/log4j/2.0/manual/api.html> > On Sep 7, 2018, at 11:38 AM, John Blum <jb...@pivotal.io> wrote: > > 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)
