I think that may be a good idea. (After we change the implementation of the logMessage method that takes a Supplier<?> to check if the supplied result is a Message to avoid wrapping it in another Message.)
Sent from my iPhone > On 2016/02/13, at 15:17, Gary Gregory <[email protected]> wrote: > >> On Fri, Feb 12, 2016 at 9:52 PM, Remko Popma <[email protected]> wrote: >> We can deprecate MessageSupplier, but it is a shame that we added 28 methods >> (4 for each log level) to the Logger interface unnecessarily... :-( > > Then we should deprecate all the APIs that use MessageSupplier as well, right? > > Gary > >> >> >>> On Sat, Feb 13, 2016 at 10:41 AM, Gary Gregory <[email protected]> >>> wrote: >>> The flip side is that code like: >>> >>> Message msg = obj instanceof Message ? (Message) obj : >>> messageFactory.newMessage(obj); >>> logMessage(... msg ...); >>> >>> smells not-OO >>> >>> Gary >>> >>> >>>> On Fri, Feb 12, 2016 at 4:45 PM, Matt Sicker <[email protected]> wrote: >>>> You could still merge them and deprecate one perhaps? >>>> >>>>> On 12 February 2016 at 18:42, Remko Popma <[email protected]> wrote: >>>>> The reason is that the Object returned by the Supplier<?> will be wrapped >>>>> in a new Message >>>>> (https://logging.apache.org/log4j/2.x/log4j-api/xref/org/apache/logging/log4j/spi/AbstractLogger.html#L1003), >>>>> while the Message returned by MessageSupplier is used as is >>>>> (https://logging.apache.org/log4j/2.x/log4j-api/xref/org/apache/logging/log4j/spi/AbstractLogger.html#L997). >>>>> >>>>> With hindsight, that could have been implemented without introducing >>>>> MessageSupplier. Darn, darn, darn! >>>>> >>>>> Could have been implemented like this: >>>>> >>>>> Object obj = supplier.get(); >>>>> Message msg = obj instanceof Message ? (Message) obj : >>>>> messageFactory.newMessage(obj); >>>>> logMessage(... msg ...); >>>>> >>>>> Why didn't I see this sooner? Mea culpa. >>>>> >>>>> >>>>> >>>>>> On Saturday, 13 February 2016, Matt Sicker <[email protected]> wrote: >>>>>> Yeah really, what was the point of MessageSupplier as a separate >>>>>> interface from Supplier<Message>? They were both added in 2.4. >>>>>> >>>>>>> On 12 February 2016 at 17:49, Gary Gregory <[email protected]> >>>>>>> wrote: >>>>>>> Hi Remko, >>>>>>> >>>>>>>> On Fri, Feb 12, 2016 at 3:31 PM, Remko Popma <[email protected]> >>>>>>>> wrote: >>>>>>>> I like the original code better since it gives more information. >>>>>>> >>>>>>> The trick is to get a message in your log that makes sense, and, in >>>>>>> this case, a debug-oriented toString() does not make sense (to me at >>>>>>> least, please see the test cases I committed today and play around). In >>>>>>> general though, FWIW, I do like toString() methods to be debug-oriented. >>>>>>> >>>>>>>> >>>>>>>> Also, if you want to provide for lamdas that supply a Message, provide >>>>>>>> a method that accepts a MessageSupplier. >>>>>>> >>>>>>> We provide both Supplier<?> and MessageSupplier APIs, I am just testing >>>>>>> it all, starting with Supplier. I'll also add MessageSupplier tests. >>>>>>> >>>>>>> What is confusing is why we have both MessageSupplier and Supplier and >>>>>>> why MessageSupplier is not defined as extending Supplier<Message>. >>>>>>> >>>>>>> Gary >>>>>>> >>>>>>>> >>>>>>>> The reason is that the Object returned by the Supplier<?> will be >>>>>>>> wrapped in a new Message >>>>>>>> (https://logging.apache.org/log4j/2.x/log4j-api/xref/org/apache/logging/log4j/spi/AbstractLogger.html#L1003), >>>>>>>> while the Message returned by MessageSupplier is used as is >>>>>>>> (https://logging.apache.org/log4j/2.x/log4j-api/xref/org/apache/logging/log4j/spi/AbstractLogger.html#L997). >>>>>>>> >>>>>>>>> On Saturday, 13 February 2016, <[email protected]> wrote: >>>>>>>>> Repository: logging-log4j2 >>>>>>>>> Updated Branches: >>>>>>>>> refs/heads/master 13f49fccc -> dc80330b5 >>>>>>>>> >>>>>>>>> >>>>>>>>> Make ObjectMessage work for code like logger.traceEntry(new >>>>>>>>> Supplier<ObjectMessage>() { ... Tests to follow but are currently >>>>>>>>> mixed >>>>>>>>> with other changes in my local repo. >>>>>>>>> >>>>>>>>> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo >>>>>>>>> Commit: >>>>>>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/dc80330b >>>>>>>>> Tree: >>>>>>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/dc80330b >>>>>>>>> Diff: >>>>>>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/dc80330b >>>>>>>>> >>>>>>>>> Branch: refs/heads/master >>>>>>>>> Commit: dc80330b521535746969e888c4b59539a147c265 >>>>>>>>> Parents: 13f49fc >>>>>>>>> Author: ggregory <[email protected]> >>>>>>>>> Authored: Fri Feb 12 11:13:03 2016 -0800 >>>>>>>>> Committer: ggregory <[email protected]> >>>>>>>>> Committed: Fri Feb 12 11:13:03 2016 -0800 >>>>>>>>> >>>>>>>>> ---------------------------------------------------------------------- >>>>>>>>> .../main/java/org/apache/logging/log4j/message/ObjectMessage.java | >>>>>>>>> 2 +- >>>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>>>>> ---------------------------------------------------------------------- >>>>>>>>> >>>>>>>>> >>>>>>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/dc80330b/log4j-api/src/main/java/org/apache/logging/log4j/message/ObjectMessage.java >>>>>>>>> ---------------------------------------------------------------------- >>>>>>>>> diff --git >>>>>>>>> a/log4j-api/src/main/java/org/apache/logging/log4j/message/ObjectMessage.java >>>>>>>>> >>>>>>>>> b/log4j-api/src/main/java/org/apache/logging/log4j/message/ObjectMessage.java >>>>>>>>> index 7cffe47..ad3dba8 100644 >>>>>>>>> --- >>>>>>>>> a/log4j-api/src/main/java/org/apache/logging/log4j/message/ObjectMessage.java >>>>>>>>> +++ >>>>>>>>> b/log4j-api/src/main/java/org/apache/logging/log4j/message/ObjectMessage.java >>>>>>>>> @@ -98,7 +98,7 @@ public class ObjectMessage implements Message { >>>>>>>>> >>>>>>>>> @Override >>>>>>>>> public String toString() { >>>>>>>>> - return "ObjectMessage[obj=" + getFormattedMessage() + ']'; >>>>>>>>> + return getFormattedMessage(); >>>>>>>>> } >>>>>>>>> >>>>>>>>> private void writeObject(final ObjectOutputStream out) throws >>>>>>>>> IOException { >>>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> E-Mail: [email protected] | [email protected] >>>>>>> Java Persistence with Hibernate, Second Edition >>>>>>> JUnit in Action, Second Edition >>>>>>> Spring Batch in Action >>>>>>> Blog: http://garygregory.wordpress.com >>>>>>> Home: http://garygregory.com/ >>>>>>> Tweet! http://twitter.com/GaryGregory >>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> Matt Sicker <[email protected]> >>>> >>>> >>>> >>>> -- >>>> Matt Sicker <[email protected]> >>> >>> >>> >>> -- >>> E-Mail: [email protected] | [email protected] >>> Java Persistence with Hibernate, Second Edition >>> JUnit in Action, Second Edition >>> Spring Batch in Action >>> Blog: http://garygregory.wordpress.com >>> Home: http://garygregory.com/ >>> Tweet! http://twitter.com/GaryGregory > > > > -- > E-Mail: [email protected] | [email protected] > Java Persistence with Hibernate, Second Edition > JUnit in Action, Second Edition > Spring Batch in Action > Blog: http://garygregory.wordpress.com > Home: http://garygregory.com/ > Tweet! http://twitter.com/GaryGregory
