Support for lazy arguments #14
Conversation
What do you think to something like this? Then, instead of:
```
if (logger.isDebugEnabled()) {
logger.debug("Message {}", SafeArg.of("name", expensiveFunc()));
}
```
I could do:
```
logger.debug("Message {}", SafeArg.of("name", () -> expensiveFunc()));
```
Log4j 2 supports something similar, but slf4j are still talking about it.
|
Might be preferable to do this in the |
|
How would that work @markelliot? At the point I call |
|
I see; I suppose this is fine, then.
|
|
our internal layout actually doesn't call so I think we need to make this change to also, I think this closes #8 |
|
Good point @gracew. Updated -> how about something like this? |
| } | ||
|
|
||
| public static <T> SafeArg<T> of(String name, T value) { | ||
| return new SafeArg<>(name, value); | ||
| return new SafeArg<>(name, () -> value); |
There was a problem hiding this comment.
Would highly recommend adding some JMH benchmarks to compare before and after here as this is going to add more allocations on potentially hot paths. I have a feeling we probably want a lazy and static impl for each safe and unsafe, and you can move most code into an abstract base class
|
|
||
| protected Arg(String name, T value) { | ||
| private boolean cached; | ||
| private T value; |
There was a problem hiding this comment.
See below, but I don't think we want to eat these extra fields for non-lazy args
|
This code is entirely unnecessary if we use a better logging api, as mentioned in the description the following already works as you have described using log4j2 log.info("Hello {}", () -> SafeArg.of("whom", "World"));This approach avoids the allocation of a new SafeArg insteace, only requiring a new supplier, where the proposed solution requires allocation of both. |
Closes #8
What do you think to something like this? Then, instead of:
I could do:
Log4j 2 supports something similar, but slf4j are still talking about it.