You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4j-dev@logging.apache.org by Gary Gregory <ga...@gmail.com> on 2016/02/15 18:06:41 UTC

MessageSupplier and Supplier

Since 2.4 we have:

public interface MessageSupplier {

    /**
     * Gets a Message.
     *
     * @return a Message
     */
    Message get();
}

and

public interface Supplier<T> {

    /**
     * Gets a value.
     *
     * @return a value
     */
    T get();
}

Which smells fishy to me. Instead I propose:

public interface MessageSupplier extends Supplier<Message> {
    // empty
}

Whether or not we do the above, we can replace:

traceExit(R, Supplier<? extends Message>)

with:

traceExit(R, MessageSupplier)

which we already have.

A test search shows the above as the only instance of "Supplier<? extends
Message>" in our Java files.

Thoughts?

-- 
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition
<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory

Re: MessageSupplier and Supplier

Posted by Gary Gregory <ga...@gmail.com>.
Done, tracked through https://issues.apache.org/jira/browse/LOG4J2-1286

Gary

On Fri, Feb 19, 2016 at 12:29 PM, Ralph Goers <ra...@dslextreme.com>
wrote:

> Yes, I am OK with this.
>
> Ralph
>
> On Feb 19, 2016, at 1:28 PM, Gary Gregory <ga...@gmail.com> wrote:
>
> If we all agree that MS should be deprecated then yes, let's have one task
> to remove new 2.6 MS APIs and deprecate existing 2.5 ones.
>
> Ralph? Are you OK with this?
>
> Then we can continue refining.
>
> Gary
> On Feb 18, 2016 8:23 AM, "Remko Popma" <re...@gmail.com> wrote:
>
>> I addressed the problem in
>> https://issues.apache.org/jira/browse/LOG4J2-1280
>> LambdaUtil now correctly handles the case where Supplier<?>s returns a
>> Message object.
>> I believe that MessageSupplier can now be deprecated.
>>
>> It may also make sense to  remove the new traceEntry/traceExit methods
>> introduced in LOG4J2-1255 that use MessageSupplier: their Supplier<?>
>> equivalent should be sufficient.
>>
>> On Tue, Feb 16, 2016 at 8:34 AM, Matt Sicker <bo...@gmail.com> wrote:
>>
>>> The wildcard bounds are checked at compile time though I thought. Plus,
>>> despite the type erasure, there's still a bit of info available through
>>> reflection, so it's not entirely lost.
>>>
>>> On 15 February 2016 at 16:23, Remko Popma <re...@gmail.com> wrote:
>>>
>>>> The wildcard bounds are no help since they get erased, so we cannot
>>>> have separate methods for Supplier<? extends Message> and Supplier<?>.
>>>> (This is why I originally introduced MessageSupplier.)
>>>>
>>>> I don't mind deprecating MessageSupplier, but the point is that in the
>>>> implementation of these logging methods we need to be very careful:
>>>>
>>>> Generally, if log(Supplier<?>) supplies an object of type Message it
>>>> doesn't need to be wrapped in an ObjectMessage, where if any other Object
>>>> is supplied it _does_ need to be wrapped.
>>>>
>>>> Similarly for where the Supplier supplies param values: if the supplied
>>>> value is a Message you need to _unwrap_ it by calling getFormattedString()
>>>> on it. The current impl of traceEntry/Exit does not do this correctly, I
>>>> reported this as a bug in LOG4J2-1255.
>>>>
>>>> Sent from my iPhone
>>>>
>>>> On 2016/02/16, at 2:21, Gary Gregory <ga...@gmail.com> wrote:
>>>>
>>>> On Mon, Feb 15, 2016 at 9:17 AM, Gary Gregory <ga...@gmail.com>
>>>> wrote:
>>>>
>>>>> On Mon, Feb 15, 2016 at 9:08 AM, Matt Sicker <bo...@gmail.com> wrote:
>>>>>
>>>>>> Sounds good. Remko was mentioning how they could be combined by
>>>>>> checking if get() returns an instanceof Message, but using the wildcard
>>>>>> bounds like you show sounds like a better way (where possible).
>>>>>>
>>>>>
>>>>> Wasn't Remko talking about a different case?
>>>>>
>>>>> Right now we have:
>>>>>
>>>>> traceExit(MessageSupplier, R)
>>>>> traceExit(Supplier<? extends Message>, R)
>>>>>
>>>>> Both "MessageSupplier.get()" and "Supplier<? extends Message>" *do*
>>>>> return a Message so I think we can remove traceExit(Supplier<? extends
>>>>> Message>, R)  (which is not even used/tested ATM).
>>>>>
>>>>
>>>> Ah, but we do not have a traceExit(Supplier<R>)
>>>>
>>>> I'll add that completeness, and also traceExit(EntryMessage). My use
>>>> cases only call for traceExit(EntryMessage, R) and traceExit(EntryMessage).
>>>>
>>>> Gary
>>>>
>>>>
>>>>> Gary
>>>>>
>>>>>>
>>>>>> On 15 February 2016 at 11:06, Gary Gregory <ga...@gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>>> Since 2.4 we have:
>>>>>>>
>>>>>>> public interface MessageSupplier {
>>>>>>>
>>>>>>>     /**
>>>>>>>      * Gets a Message.
>>>>>>>      *
>>>>>>>      * @return a Message
>>>>>>>      */
>>>>>>>     Message get();
>>>>>>> }
>>>>>>>
>>>>>>> and
>>>>>>>
>>>>>>> public interface Supplier<T> {
>>>>>>>
>>>>>>>     /**
>>>>>>>      * Gets a value.
>>>>>>>      *
>>>>>>>      * @return a value
>>>>>>>      */
>>>>>>>     T get();
>>>>>>> }
>>>>>>>
>>>>>>> Which smells fishy to me. Instead I propose:
>>>>>>>
>>>>>>> public interface MessageSupplier extends Supplier<Message> {
>>>>>>>     // empty
>>>>>>> }
>>>>>>>
>>>>>>> Whether or not we do the above, we can replace:
>>>>>>>
>>>>>>> traceExit(R, Supplier<? extends Message>)
>>>>>>>
>>>>>>> with:
>>>>>>>
>>>>>>> traceExit(R, MessageSupplier)
>>>>>>>
>>>>>>> which we already have.
>>>>>>>
>>>>>>> A test search shows the above as the only instance of "Supplier<?
>>>>>>> extends Message>" in our Java files.
>>>>>>>
>>>>>>> Thoughts?
>>>>>>>
>>>>>>> --
>>>>>>> E-Mail: garydgregory@gmail.com | ggregory@apache.org
>>>>>>> Java Persistence with Hibernate, Second Edition
>>>>>>> <http://www.manning.com/bauer3/>
>>>>>>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>>>>>>> Spring Batch in Action <http://www.manning.com/templier/>
>>>>>>> Blog: http://garygregory.wordpress.com
>>>>>>> Home: http://garygregory.com/
>>>>>>> Tweet! http://twitter.com/GaryGregory
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Matt Sicker <bo...@gmail.com>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> E-Mail: garydgregory@gmail.com | ggregory@apache.org
>>>>> Java Persistence with Hibernate, Second Edition
>>>>> <http://www.manning.com/bauer3/>
>>>>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>>>>> Spring Batch in Action <http://www.manning.com/templier/>
>>>>> Blog: http://garygregory.wordpress.com
>>>>> Home: http://garygregory.com/
>>>>> Tweet! http://twitter.com/GaryGregory
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> E-Mail: garydgregory@gmail.com | ggregory@apache.org
>>>> Java Persistence with Hibernate, Second Edition
>>>> <http://www.manning.com/bauer3/>
>>>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>>>> Spring Batch in Action <http://www.manning.com/templier/>
>>>> Blog: http://garygregory.wordpress.com
>>>> Home: http://garygregory.com/
>>>> Tweet! http://twitter.com/GaryGregory
>>>>
>>>>
>>>
>>>
>>> --
>>> Matt Sicker <bo...@gmail.com>
>>>
>>
>>
>


-- 
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition
<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory

Re: MessageSupplier and Supplier

Posted by Ralph Goers <ra...@dslextreme.com>.
Yes, I am OK with this.

Ralph

> On Feb 19, 2016, at 1:28 PM, Gary Gregory <ga...@gmail.com> wrote:
> 
> If we all agree that MS should be deprecated then yes, let's have one task to remove new 2.6 MS APIs and deprecate existing 2.5 ones.
> 
> Ralph? Are you OK with this?
> 
> Then we can continue refining.
> 
> Gary
> 
> On Feb 18, 2016 8:23 AM, "Remko Popma" <remko.popma@gmail.com <ma...@gmail.com>> wrote:
> I addressed the problem in https://issues.apache.org/jira/browse/LOG4J2-1280 <https://issues.apache.org/jira/browse/LOG4J2-1280>
> LambdaUtil now correctly handles the case where Supplier<?>s returns a Message object.
> I believe that MessageSupplier can now be deprecated.
> 
> It may also make sense to  remove the new traceEntry/traceExit methods introduced in LOG4J2-1255 that use MessageSupplier: their Supplier<?> equivalent should be sufficient.
> 
> On Tue, Feb 16, 2016 at 8:34 AM, Matt Sicker <boards@gmail.com <ma...@gmail.com>> wrote:
> The wildcard bounds are checked at compile time though I thought. Plus, despite the type erasure, there's still a bit of info available through reflection, so it's not entirely lost.
> 
> On 15 February 2016 at 16:23, Remko Popma <remko.popma@gmail.com <ma...@gmail.com>> wrote:
> The wildcard bounds are no help since they get erased, so we cannot have separate methods for Supplier<? extends Message> and Supplier<?>. (This is why I originally introduced MessageSupplier.) 
> 
> I don't mind deprecating MessageSupplier, but the point is that in the implementation of these logging methods we need to be very careful: 
> 
> Generally, if log(Supplier<?>) supplies an object of type Message it doesn't need to be wrapped in an ObjectMessage, where if any other Object is supplied it _does_ need to be wrapped. 
> 
> Similarly for where the Supplier supplies param values: if the supplied value is a Message you need to _unwrap_ it by calling getFormattedString() on it. The current impl of traceEntry/Exit does not do this correctly, I reported this as a bug in LOG4J2-1255. 
> 
> Sent from my iPhone
> 
> On 2016/02/16, at 2:21, Gary Gregory <garydgregory@gmail.com <ma...@gmail.com>> wrote:
> 
>> On Mon, Feb 15, 2016 at 9:17 AM, Gary Gregory <garydgregory@gmail.com <ma...@gmail.com>> wrote:
>> On Mon, Feb 15, 2016 at 9:08 AM, Matt Sicker <boards@gmail.com <ma...@gmail.com>> wrote:
>> Sounds good. Remko was mentioning how they could be combined by checking if get() returns an instanceof Message, but using the wildcard bounds like you show sounds like a better way (where possible).
>> 
>> Wasn't Remko talking about a different case? 
>> 
>> Right now we have:
>> 
>> traceExit(MessageSupplier, R)
>> traceExit(Supplier<? extends Message>, R) 
>> 
>> Both "MessageSupplier.get()" and "Supplier<? extends Message>" *do* return a Message so I think we can remove traceExit(Supplier<? extends Message>, R)  (which is not even used/tested ATM).
>> 
>> Ah, but we do not have a traceExit(Supplier<R>) 
>> 
>> I'll add that completeness, and also traceExit(EntryMessage). My use cases only call for traceExit(EntryMessage, R) and traceExit(EntryMessage).
>> 
>> Gary
>> 
>> 
>> Gary
>> 
>> On 15 February 2016 at 11:06, Gary Gregory <garydgregory@gmail.com <ma...@gmail.com>> wrote:
>> Since 2.4 we have:
>> 
>> public interface MessageSupplier {
>> 
>>     /**
>>      * Gets a Message.
>>      *
>>      * @return a Message
>>      */
>>     Message get();
>> }
>> 
>> and
>> 
>> public interface Supplier<T> {
>> 
>>     /**
>>      * Gets a value.
>>      *
>>      * @return a value
>>      */
>>     T get();
>> }
>> 
>> Which smells fishy to me. Instead I propose:
>> 
>> public interface MessageSupplier extends Supplier<Message> {
>>     // empty
>> }
>> 
>> Whether or not we do the above, we can replace:
>> 
>> traceExit(R, Supplier<? extends Message>)
>> 
>> with:
>> 
>> traceExit(R, MessageSupplier)
>> 
>> which we already have.
>> 
>> A test search shows the above as the only instance of "Supplier<? extends Message>" in our Java files.
>> 
>> Thoughts?
>> 
>> -- 
>> E-Mail: garydgregory@gmail.com <ma...@gmail.com> | ggregory@apache.org  <ma...@apache.org>
>> Java Persistence with Hibernate, Second Edition <http://www.manning.com/bauer3/>
>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>> Spring Batch in Action <http://www.manning.com/templier/>
>> Blog: http://garygregory.wordpress.com <http://garygregory.wordpress.com/> 
>> Home: http://garygregory.com/ <http://garygregory.com/>
>> Tweet! http://twitter.com/GaryGregory <http://twitter.com/GaryGregory>
>> 
>> 
>> -- 
>> Matt Sicker <boards@gmail.com <ma...@gmail.com>>
>> 
>> 
>> 
>> -- 
>> E-Mail: garydgregory@gmail.com <ma...@gmail.com> | ggregory@apache.org  <ma...@apache.org>
>> Java Persistence with Hibernate, Second Edition <http://www.manning.com/bauer3/>
>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>> Spring Batch in Action <http://www.manning.com/templier/>
>> Blog: http://garygregory.wordpress.com <http://garygregory.wordpress.com/> 
>> Home: http://garygregory.com/ <http://garygregory.com/>
>> Tweet! http://twitter.com/GaryGregory <http://twitter.com/GaryGregory>
>> 
>> 
>> -- 
>> E-Mail: garydgregory@gmail.com <ma...@gmail.com> | ggregory@apache.org  <ma...@apache.org>
>> Java Persistence with Hibernate, Second Edition <http://www.manning.com/bauer3/>
>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>> Spring Batch in Action <http://www.manning.com/templier/>
>> Blog: http://garygregory.wordpress.com <http://garygregory.wordpress.com/> 
>> Home: http://garygregory.com/ <http://garygregory.com/>
>> Tweet! http://twitter.com/GaryGregory <http://twitter.com/GaryGregory>
> 
> 
> -- 
> Matt Sicker <boards@gmail.com <ma...@gmail.com>>
> 


Re: MessageSupplier and Supplier

Posted by Gary Gregory <ga...@gmail.com>.
If we all agree that MS should be deprecated then yes, let's have one task
to remove new 2.6 MS APIs and deprecate existing 2.5 ones.

Ralph? Are you OK with this?

Then we can continue refining.

Gary
On Feb 18, 2016 8:23 AM, "Remko Popma" <re...@gmail.com> wrote:

> I addressed the problem in
> https://issues.apache.org/jira/browse/LOG4J2-1280
> LambdaUtil now correctly handles the case where Supplier<?>s returns a
> Message object.
> I believe that MessageSupplier can now be deprecated.
>
> It may also make sense to  remove the new traceEntry/traceExit methods
> introduced in LOG4J2-1255 that use MessageSupplier: their Supplier<?>
> equivalent should be sufficient.
>
> On Tue, Feb 16, 2016 at 8:34 AM, Matt Sicker <bo...@gmail.com> wrote:
>
>> The wildcard bounds are checked at compile time though I thought. Plus,
>> despite the type erasure, there's still a bit of info available through
>> reflection, so it's not entirely lost.
>>
>> On 15 February 2016 at 16:23, Remko Popma <re...@gmail.com> wrote:
>>
>>> The wildcard bounds are no help since they get erased, so we cannot have
>>> separate methods for Supplier<? extends Message> and Supplier<?>. (This is
>>> why I originally introduced MessageSupplier.)
>>>
>>> I don't mind deprecating MessageSupplier, but the point is that in the
>>> implementation of these logging methods we need to be very careful:
>>>
>>> Generally, if log(Supplier<?>) supplies an object of type Message it
>>> doesn't need to be wrapped in an ObjectMessage, where if any other Object
>>> is supplied it _does_ need to be wrapped.
>>>
>>> Similarly for where the Supplier supplies param values: if the supplied
>>> value is a Message you need to _unwrap_ it by calling getFormattedString()
>>> on it. The current impl of traceEntry/Exit does not do this correctly, I
>>> reported this as a bug in LOG4J2-1255.
>>>
>>> Sent from my iPhone
>>>
>>> On 2016/02/16, at 2:21, Gary Gregory <ga...@gmail.com> wrote:
>>>
>>> On Mon, Feb 15, 2016 at 9:17 AM, Gary Gregory <ga...@gmail.com>
>>> wrote:
>>>
>>>> On Mon, Feb 15, 2016 at 9:08 AM, Matt Sicker <bo...@gmail.com> wrote:
>>>>
>>>>> Sounds good. Remko was mentioning how they could be combined by
>>>>> checking if get() returns an instanceof Message, but using the wildcard
>>>>> bounds like you show sounds like a better way (where possible).
>>>>>
>>>>
>>>> Wasn't Remko talking about a different case?
>>>>
>>>> Right now we have:
>>>>
>>>> traceExit(MessageSupplier, R)
>>>> traceExit(Supplier<? extends Message>, R)
>>>>
>>>> Both "MessageSupplier.get()" and "Supplier<? extends Message>" *do*
>>>> return a Message so I think we can remove traceExit(Supplier<? extends
>>>> Message>, R)  (which is not even used/tested ATM).
>>>>
>>>
>>> Ah, but we do not have a traceExit(Supplier<R>)
>>>
>>> I'll add that completeness, and also traceExit(EntryMessage). My use
>>> cases only call for traceExit(EntryMessage, R) and traceExit(EntryMessage).
>>>
>>> Gary
>>>
>>>
>>>> Gary
>>>>
>>>>>
>>>>> On 15 February 2016 at 11:06, Gary Gregory <ga...@gmail.com>
>>>>> wrote:
>>>>>
>>>>>> Since 2.4 we have:
>>>>>>
>>>>>> public interface MessageSupplier {
>>>>>>
>>>>>>     /**
>>>>>>      * Gets a Message.
>>>>>>      *
>>>>>>      * @return a Message
>>>>>>      */
>>>>>>     Message get();
>>>>>> }
>>>>>>
>>>>>> and
>>>>>>
>>>>>> public interface Supplier<T> {
>>>>>>
>>>>>>     /**
>>>>>>      * Gets a value.
>>>>>>      *
>>>>>>      * @return a value
>>>>>>      */
>>>>>>     T get();
>>>>>> }
>>>>>>
>>>>>> Which smells fishy to me. Instead I propose:
>>>>>>
>>>>>> public interface MessageSupplier extends Supplier<Message> {
>>>>>>     // empty
>>>>>> }
>>>>>>
>>>>>> Whether or not we do the above, we can replace:
>>>>>>
>>>>>> traceExit(R, Supplier<? extends Message>)
>>>>>>
>>>>>> with:
>>>>>>
>>>>>> traceExit(R, MessageSupplier)
>>>>>>
>>>>>> which we already have.
>>>>>>
>>>>>> A test search shows the above as the only instance of "Supplier<?
>>>>>> extends Message>" in our Java files.
>>>>>>
>>>>>> Thoughts?
>>>>>>
>>>>>> --
>>>>>> E-Mail: garydgregory@gmail.com | ggregory@apache.org
>>>>>> Java Persistence with Hibernate, Second Edition
>>>>>> <http://www.manning.com/bauer3/>
>>>>>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>>>>>> Spring Batch in Action <http://www.manning.com/templier/>
>>>>>> Blog: http://garygregory.wordpress.com
>>>>>> Home: http://garygregory.com/
>>>>>> Tweet! http://twitter.com/GaryGregory
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Matt Sicker <bo...@gmail.com>
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> E-Mail: garydgregory@gmail.com | ggregory@apache.org
>>>> Java Persistence with Hibernate, Second Edition
>>>> <http://www.manning.com/bauer3/>
>>>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>>>> Spring Batch in Action <http://www.manning.com/templier/>
>>>> Blog: http://garygregory.wordpress.com
>>>> Home: http://garygregory.com/
>>>> Tweet! http://twitter.com/GaryGregory
>>>>
>>>
>>>
>>>
>>> --
>>> E-Mail: garydgregory@gmail.com | ggregory@apache.org
>>> Java Persistence with Hibernate, Second Edition
>>> <http://www.manning.com/bauer3/>
>>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>>> Spring Batch in Action <http://www.manning.com/templier/>
>>> Blog: http://garygregory.wordpress.com
>>> Home: http://garygregory.com/
>>> Tweet! http://twitter.com/GaryGregory
>>>
>>>
>>
>>
>> --
>> Matt Sicker <bo...@gmail.com>
>>
>
>

Re: MessageSupplier and Supplier

Posted by Remko Popma <re...@gmail.com>.
I addressed the problem in https://issues.apache.org/jira/browse/LOG4J2-1280
LambdaUtil now correctly handles the case where Supplier<?>s returns a
Message object.
I believe that MessageSupplier can now be deprecated.

It may also make sense to  remove the new traceEntry/traceExit methods
introduced in LOG4J2-1255 that use MessageSupplier: their Supplier<?>
equivalent should be sufficient.

On Tue, Feb 16, 2016 at 8:34 AM, Matt Sicker <bo...@gmail.com> wrote:

> The wildcard bounds are checked at compile time though I thought. Plus,
> despite the type erasure, there's still a bit of info available through
> reflection, so it's not entirely lost.
>
> On 15 February 2016 at 16:23, Remko Popma <re...@gmail.com> wrote:
>
>> The wildcard bounds are no help since they get erased, so we cannot have
>> separate methods for Supplier<? extends Message> and Supplier<?>. (This is
>> why I originally introduced MessageSupplier.)
>>
>> I don't mind deprecating MessageSupplier, but the point is that in the
>> implementation of these logging methods we need to be very careful:
>>
>> Generally, if log(Supplier<?>) supplies an object of type Message it
>> doesn't need to be wrapped in an ObjectMessage, where if any other Object
>> is supplied it _does_ need to be wrapped.
>>
>> Similarly for where the Supplier supplies param values: if the supplied
>> value is a Message you need to _unwrap_ it by calling getFormattedString()
>> on it. The current impl of traceEntry/Exit does not do this correctly, I
>> reported this as a bug in LOG4J2-1255.
>>
>> Sent from my iPhone
>>
>> On 2016/02/16, at 2:21, Gary Gregory <ga...@gmail.com> wrote:
>>
>> On Mon, Feb 15, 2016 at 9:17 AM, Gary Gregory <ga...@gmail.com>
>> wrote:
>>
>>> On Mon, Feb 15, 2016 at 9:08 AM, Matt Sicker <bo...@gmail.com> wrote:
>>>
>>>> Sounds good. Remko was mentioning how they could be combined by
>>>> checking if get() returns an instanceof Message, but using the wildcard
>>>> bounds like you show sounds like a better way (where possible).
>>>>
>>>
>>> Wasn't Remko talking about a different case?
>>>
>>> Right now we have:
>>>
>>> traceExit(MessageSupplier, R)
>>> traceExit(Supplier<? extends Message>, R)
>>>
>>> Both "MessageSupplier.get()" and "Supplier<? extends Message>" *do*
>>> return a Message so I think we can remove traceExit(Supplier<? extends
>>> Message>, R)  (which is not even used/tested ATM).
>>>
>>
>> Ah, but we do not have a traceExit(Supplier<R>)
>>
>> I'll add that completeness, and also traceExit(EntryMessage). My use
>> cases only call for traceExit(EntryMessage, R) and traceExit(EntryMessage).
>>
>> Gary
>>
>>
>>> Gary
>>>
>>>>
>>>> On 15 February 2016 at 11:06, Gary Gregory <ga...@gmail.com>
>>>> wrote:
>>>>
>>>>> Since 2.4 we have:
>>>>>
>>>>> public interface MessageSupplier {
>>>>>
>>>>>     /**
>>>>>      * Gets a Message.
>>>>>      *
>>>>>      * @return a Message
>>>>>      */
>>>>>     Message get();
>>>>> }
>>>>>
>>>>> and
>>>>>
>>>>> public interface Supplier<T> {
>>>>>
>>>>>     /**
>>>>>      * Gets a value.
>>>>>      *
>>>>>      * @return a value
>>>>>      */
>>>>>     T get();
>>>>> }
>>>>>
>>>>> Which smells fishy to me. Instead I propose:
>>>>>
>>>>> public interface MessageSupplier extends Supplier<Message> {
>>>>>     // empty
>>>>> }
>>>>>
>>>>> Whether or not we do the above, we can replace:
>>>>>
>>>>> traceExit(R, Supplier<? extends Message>)
>>>>>
>>>>> with:
>>>>>
>>>>> traceExit(R, MessageSupplier)
>>>>>
>>>>> which we already have.
>>>>>
>>>>> A test search shows the above as the only instance of "Supplier<?
>>>>> extends Message>" in our Java files.
>>>>>
>>>>> Thoughts?
>>>>>
>>>>> --
>>>>> E-Mail: garydgregory@gmail.com | ggregory@apache.org
>>>>> Java Persistence with Hibernate, Second Edition
>>>>> <http://www.manning.com/bauer3/>
>>>>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>>>>> Spring Batch in Action <http://www.manning.com/templier/>
>>>>> Blog: http://garygregory.wordpress.com
>>>>> Home: http://garygregory.com/
>>>>> Tweet! http://twitter.com/GaryGregory
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Matt Sicker <bo...@gmail.com>
>>>>
>>>
>>>
>>>
>>> --
>>> E-Mail: garydgregory@gmail.com | ggregory@apache.org
>>> Java Persistence with Hibernate, Second Edition
>>> <http://www.manning.com/bauer3/>
>>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>>> Spring Batch in Action <http://www.manning.com/templier/>
>>> Blog: http://garygregory.wordpress.com
>>> Home: http://garygregory.com/
>>> Tweet! http://twitter.com/GaryGregory
>>>
>>
>>
>>
>> --
>> E-Mail: garydgregory@gmail.com | ggregory@apache.org
>> Java Persistence with Hibernate, Second Edition
>> <http://www.manning.com/bauer3/>
>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>> Spring Batch in Action <http://www.manning.com/templier/>
>> Blog: http://garygregory.wordpress.com
>> Home: http://garygregory.com/
>> Tweet! http://twitter.com/GaryGregory
>>
>>
>
>
> --
> Matt Sicker <bo...@gmail.com>
>

Re: MessageSupplier and Supplier

Posted by Matt Sicker <bo...@gmail.com>.
The wildcard bounds are checked at compile time though I thought. Plus,
despite the type erasure, there's still a bit of info available through
reflection, so it's not entirely lost.

On 15 February 2016 at 16:23, Remko Popma <re...@gmail.com> wrote:

> The wildcard bounds are no help since they get erased, so we cannot have
> separate methods for Supplier<? extends Message> and Supplier<?>. (This is
> why I originally introduced MessageSupplier.)
>
> I don't mind deprecating MessageSupplier, but the point is that in the
> implementation of these logging methods we need to be very careful:
>
> Generally, if log(Supplier<?>) supplies an object of type Message it
> doesn't need to be wrapped in an ObjectMessage, where if any other Object
> is supplied it _does_ need to be wrapped.
>
> Similarly for where the Supplier supplies param values: if the supplied
> value is a Message you need to _unwrap_ it by calling getFormattedString()
> on it. The current impl of traceEntry/Exit does not do this correctly, I
> reported this as a bug in LOG4J2-1255.
>
> Sent from my iPhone
>
> On 2016/02/16, at 2:21, Gary Gregory <ga...@gmail.com> wrote:
>
> On Mon, Feb 15, 2016 at 9:17 AM, Gary Gregory <ga...@gmail.com>
> wrote:
>
>> On Mon, Feb 15, 2016 at 9:08 AM, Matt Sicker <bo...@gmail.com> wrote:
>>
>>> Sounds good. Remko was mentioning how they could be combined by checking
>>> if get() returns an instanceof Message, but using the wildcard bounds like
>>> you show sounds like a better way (where possible).
>>>
>>
>> Wasn't Remko talking about a different case?
>>
>> Right now we have:
>>
>> traceExit(MessageSupplier, R)
>> traceExit(Supplier<? extends Message>, R)
>>
>> Both "MessageSupplier.get()" and "Supplier<? extends Message>" *do*
>> return a Message so I think we can remove traceExit(Supplier<? extends
>> Message>, R)  (which is not even used/tested ATM).
>>
>
> Ah, but we do not have a traceExit(Supplier<R>)
>
> I'll add that completeness, and also traceExit(EntryMessage). My use cases
> only call for traceExit(EntryMessage, R) and traceExit(EntryMessage).
>
> Gary
>
>
>> Gary
>>
>>>
>>> On 15 February 2016 at 11:06, Gary Gregory <ga...@gmail.com>
>>> wrote:
>>>
>>>> Since 2.4 we have:
>>>>
>>>> public interface MessageSupplier {
>>>>
>>>>     /**
>>>>      * Gets a Message.
>>>>      *
>>>>      * @return a Message
>>>>      */
>>>>     Message get();
>>>> }
>>>>
>>>> and
>>>>
>>>> public interface Supplier<T> {
>>>>
>>>>     /**
>>>>      * Gets a value.
>>>>      *
>>>>      * @return a value
>>>>      */
>>>>     T get();
>>>> }
>>>>
>>>> Which smells fishy to me. Instead I propose:
>>>>
>>>> public interface MessageSupplier extends Supplier<Message> {
>>>>     // empty
>>>> }
>>>>
>>>> Whether or not we do the above, we can replace:
>>>>
>>>> traceExit(R, Supplier<? extends Message>)
>>>>
>>>> with:
>>>>
>>>> traceExit(R, MessageSupplier)
>>>>
>>>> which we already have.
>>>>
>>>> A test search shows the above as the only instance of "Supplier<?
>>>> extends Message>" in our Java files.
>>>>
>>>> Thoughts?
>>>>
>>>> --
>>>> E-Mail: garydgregory@gmail.com | ggregory@apache.org
>>>> Java Persistence with Hibernate, Second Edition
>>>> <http://www.manning.com/bauer3/>
>>>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>>>> Spring Batch in Action <http://www.manning.com/templier/>
>>>> Blog: http://garygregory.wordpress.com
>>>> Home: http://garygregory.com/
>>>> Tweet! http://twitter.com/GaryGregory
>>>>
>>>
>>>
>>>
>>> --
>>> Matt Sicker <bo...@gmail.com>
>>>
>>
>>
>>
>> --
>> E-Mail: garydgregory@gmail.com | ggregory@apache.org
>> Java Persistence with Hibernate, Second Edition
>> <http://www.manning.com/bauer3/>
>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>> Spring Batch in Action <http://www.manning.com/templier/>
>> Blog: http://garygregory.wordpress.com
>> Home: http://garygregory.com/
>> Tweet! http://twitter.com/GaryGregory
>>
>
>
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@apache.org
> Java Persistence with Hibernate, Second Edition
> <http://www.manning.com/bauer3/>
> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> Spring Batch in Action <http://www.manning.com/templier/>
> Blog: http://garygregory.wordpress.com
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory
>
>


-- 
Matt Sicker <bo...@gmail.com>

Re: MessageSupplier and Supplier

Posted by Remko Popma <re...@gmail.com>.
The wildcard bounds are no help since they get erased, so we cannot have separate methods for Supplier<? extends Message> and Supplier<?>. (This is why I originally introduced MessageSupplier.) 

I don't mind deprecating MessageSupplier, but the point is that in the implementation of these logging methods we need to be very careful: 

Generally, if log(Supplier<?>) supplies an object of type Message it doesn't need to be wrapped in an ObjectMessage, where if any other Object is supplied it _does_ need to be wrapped. 

Similarly for where the Supplier supplies param values: if the supplied value is a Message you need to _unwrap_ it by calling getFormattedString() on it. The current impl of traceEntry/Exit does not do this correctly, I reported this as a bug in LOG4J2-1255. 

Sent from my iPhone

> On 2016/02/16, at 2:21, Gary Gregory <ga...@gmail.com> wrote:
> 
>> On Mon, Feb 15, 2016 at 9:17 AM, Gary Gregory <ga...@gmail.com> wrote:
>>> On Mon, Feb 15, 2016 at 9:08 AM, Matt Sicker <bo...@gmail.com> wrote:
>>> Sounds good. Remko was mentioning how they could be combined by checking if get() returns an instanceof Message, but using the wildcard bounds like you show sounds like a better way (where possible).
>> 
>> Wasn't Remko talking about a different case? 
>> 
>> Right now we have:
>> 
>> traceExit(MessageSupplier, R)
>> traceExit(Supplier<? extends Message>, R) 
>> 
>> Both "MessageSupplier.get()" and "Supplier<? extends Message>" *do* return a Message so I think we can remove traceExit(Supplier<? extends Message>, R)  (which is not even used/tested ATM).
> 
> Ah, but we do not have a traceExit(Supplier<R>) 
> 
> I'll add that completeness, and also traceExit(EntryMessage). My use cases only call for traceExit(EntryMessage, R) and traceExit(EntryMessage).
> 
> Gary
> 
>> 
>> Gary
>>> 
>>>> On 15 February 2016 at 11:06, Gary Gregory <ga...@gmail.com> wrote:
>>>> Since 2.4 we have:
>>>> 
>>>> public interface MessageSupplier {
>>>> 
>>>>     /**
>>>>      * Gets a Message.
>>>>      *
>>>>      * @return a Message
>>>>      */
>>>>     Message get();
>>>> }
>>>> 
>>>> and
>>>> 
>>>> public interface Supplier<T> {
>>>> 
>>>>     /**
>>>>      * Gets a value.
>>>>      *
>>>>      * @return a value
>>>>      */
>>>>     T get();
>>>> }
>>>> 
>>>> Which smells fishy to me. Instead I propose:
>>>> 
>>>> public interface MessageSupplier extends Supplier<Message> {
>>>>     // empty
>>>> }
>>>> 
>>>> Whether or not we do the above, we can replace:
>>>> 
>>>> traceExit(R, Supplier<? extends Message>)
>>>> 
>>>> with:
>>>> 
>>>> traceExit(R, MessageSupplier)
>>>> 
>>>> which we already have.
>>>> 
>>>> A test search shows the above as the only instance of "Supplier<? extends Message>" in our Java files.
>>>> 
>>>> Thoughts?
>>>> 
>>>> -- 
>>>> E-Mail: garydgregory@gmail.com | ggregory@apache.org 
>>>> 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 <bo...@gmail.com>
>> 
>> 
>> 
>> 
>> -- 
>> E-Mail: garydgregory@gmail.com | ggregory@apache.org 
>> 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: garydgregory@gmail.com | ggregory@apache.org 
> 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

Re: MessageSupplier and Supplier

Posted by Gary Gregory <ga...@gmail.com>.
On Mon, Feb 15, 2016 at 9:17 AM, Gary Gregory <ga...@gmail.com>
wrote:

> On Mon, Feb 15, 2016 at 9:08 AM, Matt Sicker <bo...@gmail.com> wrote:
>
>> Sounds good. Remko was mentioning how they could be combined by checking
>> if get() returns an instanceof Message, but using the wildcard bounds like
>> you show sounds like a better way (where possible).
>>
>
> Wasn't Remko talking about a different case?
>
> Right now we have:
>
> traceExit(MessageSupplier, R)
> traceExit(Supplier<? extends Message>, R)
>
> Both "MessageSupplier.get()" and "Supplier<? extends Message>" *do* return
> a Message so I think we can remove traceExit(Supplier<? extends Message>,
> R)  (which is not even used/tested ATM).
>

Ah, but we do not have a traceExit(Supplier<R>)

I'll add that completeness, and also traceExit(EntryMessage). My use cases
only call for traceExit(EntryMessage, R) and traceExit(EntryMessage).

Gary


> Gary
>
>>
>> On 15 February 2016 at 11:06, Gary Gregory <ga...@gmail.com>
>> wrote:
>>
>>> Since 2.4 we have:
>>>
>>> public interface MessageSupplier {
>>>
>>>     /**
>>>      * Gets a Message.
>>>      *
>>>      * @return a Message
>>>      */
>>>     Message get();
>>> }
>>>
>>> and
>>>
>>> public interface Supplier<T> {
>>>
>>>     /**
>>>      * Gets a value.
>>>      *
>>>      * @return a value
>>>      */
>>>     T get();
>>> }
>>>
>>> Which smells fishy to me. Instead I propose:
>>>
>>> public interface MessageSupplier extends Supplier<Message> {
>>>     // empty
>>> }
>>>
>>> Whether or not we do the above, we can replace:
>>>
>>> traceExit(R, Supplier<? extends Message>)
>>>
>>> with:
>>>
>>> traceExit(R, MessageSupplier)
>>>
>>> which we already have.
>>>
>>> A test search shows the above as the only instance of "Supplier<?
>>> extends Message>" in our Java files.
>>>
>>> Thoughts?
>>>
>>> --
>>> E-Mail: garydgregory@gmail.com | ggregory@apache.org
>>> Java Persistence with Hibernate, Second Edition
>>> <http://www.manning.com/bauer3/>
>>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>>> Spring Batch in Action <http://www.manning.com/templier/>
>>> Blog: http://garygregory.wordpress.com
>>> Home: http://garygregory.com/
>>> Tweet! http://twitter.com/GaryGregory
>>>
>>
>>
>>
>> --
>> Matt Sicker <bo...@gmail.com>
>>
>
>
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@apache.org
> Java Persistence with Hibernate, Second Edition
> <http://www.manning.com/bauer3/>
> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> Spring Batch in Action <http://www.manning.com/templier/>
> Blog: http://garygregory.wordpress.com
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory
>



-- 
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition
<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory

Re: MessageSupplier and Supplier

Posted by Gary Gregory <ga...@gmail.com>.
On Mon, Feb 15, 2016 at 9:08 AM, Matt Sicker <bo...@gmail.com> wrote:

> Sounds good. Remko was mentioning how they could be combined by checking
> if get() returns an instanceof Message, but using the wildcard bounds like
> you show sounds like a better way (where possible).
>

Wasn't Remko talking about a different case?

Right now we have:

traceExit(MessageSupplier, R)
traceExit(Supplier<? extends Message>, R)

Both "MessageSupplier.get()" and "Supplier<? extends Message>" *do* return
a Message so I think we can remove traceExit(Supplier<? extends Message>,
R)  (which is not even used/tested ATM).

Gary

>
> On 15 February 2016 at 11:06, Gary Gregory <ga...@gmail.com> wrote:
>
>> Since 2.4 we have:
>>
>> public interface MessageSupplier {
>>
>>     /**
>>      * Gets a Message.
>>      *
>>      * @return a Message
>>      */
>>     Message get();
>> }
>>
>> and
>>
>> public interface Supplier<T> {
>>
>>     /**
>>      * Gets a value.
>>      *
>>      * @return a value
>>      */
>>     T get();
>> }
>>
>> Which smells fishy to me. Instead I propose:
>>
>> public interface MessageSupplier extends Supplier<Message> {
>>     // empty
>> }
>>
>> Whether or not we do the above, we can replace:
>>
>> traceExit(R, Supplier<? extends Message>)
>>
>> with:
>>
>> traceExit(R, MessageSupplier)
>>
>> which we already have.
>>
>> A test search shows the above as the only instance of "Supplier<? extends
>> Message>" in our Java files.
>>
>> Thoughts?
>>
>> --
>> E-Mail: garydgregory@gmail.com | ggregory@apache.org
>> Java Persistence with Hibernate, Second Edition
>> <http://www.manning.com/bauer3/>
>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>> Spring Batch in Action <http://www.manning.com/templier/>
>> Blog: http://garygregory.wordpress.com
>> Home: http://garygregory.com/
>> Tweet! http://twitter.com/GaryGregory
>>
>
>
>
> --
> Matt Sicker <bo...@gmail.com>
>



-- 
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition
<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory

Re: MessageSupplier and Supplier

Posted by Matt Sicker <bo...@gmail.com>.
Sounds good. Remko was mentioning how they could be combined by checking if
get() returns an instanceof Message, but using the wildcard bounds like you
show sounds like a better way (where possible).

On 15 February 2016 at 11:06, Gary Gregory <ga...@gmail.com> wrote:

> Since 2.4 we have:
>
> public interface MessageSupplier {
>
>     /**
>      * Gets a Message.
>      *
>      * @return a Message
>      */
>     Message get();
> }
>
> and
>
> public interface Supplier<T> {
>
>     /**
>      * Gets a value.
>      *
>      * @return a value
>      */
>     T get();
> }
>
> Which smells fishy to me. Instead I propose:
>
> public interface MessageSupplier extends Supplier<Message> {
>     // empty
> }
>
> Whether or not we do the above, we can replace:
>
> traceExit(R, Supplier<? extends Message>)
>
> with:
>
> traceExit(R, MessageSupplier)
>
> which we already have.
>
> A test search shows the above as the only instance of "Supplier<? extends
> Message>" in our Java files.
>
> Thoughts?
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@apache.org
> Java Persistence with Hibernate, Second Edition
> <http://www.manning.com/bauer3/>
> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> Spring Batch in Action <http://www.manning.com/templier/>
> Blog: http://garygregory.wordpress.com
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory
>



-- 
Matt Sicker <bo...@gmail.com>