You getObject method below returns a String. That is why I said the method
isn’t needed.
Ralph
> On Mar 25, 2021, at 11:31 AM, Tim Perry <tim.v...@gmail.com> wrote:
>
> If we are only going to allow strings in the map, then we don't need a new
> method.
>
> If we are going to allow other values in the map, then we would need a new
> method.
>
> Maybe I lost the plot, but I don't see the point of allowing people to put
> non-string objects into the map unless they can get non-string objects back.
>
> On Thu, Mar 25, 2021 at 10:51 AM Ralph Goers <ralph.go...@dslextreme.com>
> wrote:
>
>> The existing get method does a deep toString on the value object already.
>> I’m not sure why we would need a new method if the return value is a String.
>>
>> Ralph
>>
>>> On Mar 25, 2021, at 10:28 AM, Tim Perry <tim.v...@gmail.com> wrote:
>>>
>>> Would we also have
>>> public String getObject(String) {...}
>>>
>>> What will happen when someone calls get and the object that key is mapped
>>> to is not a String? Does it return null? Throw an exception?
>>>
>>> Instead of allowing non-string values maybe change the type of data to
>>> Map<String, String> instead of <String, Object> and avoid the mess?
>>>
>>>
>>> On Thu, Mar 25, 2021 at 8:02 AM Gary Gregory <garydgreg...@gmail.com>
>> wrote:
>>>
>>>> The 2 postfix gives me flashbacks of Microsoft COM interfaces! :-p
>>>> putObject is less shudder inducing than put2... for me at least.
>>>>
>>>> Gary
>>>>
>>>>
>>>> On Thu, Mar 25, 2021, 01:08 Remko Popma <remko.po...@gmail.com> wrote:
>>>>
>>>>> Haha!
>>>>> putObject?
>>>>>
>>>>>
>>>>>
>>>>>> On Mar 25, 2021, at 11:39, Ralph Goers <ralph.go...@dslextreme.com>
>>>>> wrote:
>>>>>>
>>>>>> I’m sure that will drive Gary nuts. Let’s call the new method
>>>> “put2()”.
>>>>>>
>>>>>> Ralph
>>>>>>
>>>>>>> On Mar 24, 2021, at 5:18 PM, Remko Popma <remko.po...@gmail.com>
>>>> wrote:
>>>>>>>
>>>>>>> Instead of overloading the existing method(s), how about adding new
>>>>> methods
>>>>>>> with a slightly different name that takes Object parameters?
>>>>>>>
>>>>>>>> On Thu, Mar 25, 2021 at 8:46 AM Carter Kozak <cko...@ckozak.net>
>>>>> wrote:
>>>>>>>>
>>>>>>>> The method argument type changes are an ABI break, but I recall
>>>>> extending
>>>>>>>> MapMessage within a project in order to support more expressive
>> types
>>>>> --
>>>>>>>> that
>>>>>>>> may have relied on dangerously casting the result of
>>>>>>>> getIndexedReadOnlyStringMap
>>>>>>>> to an IndexedStringMap.
>>>>>>>> Including a safer Object-valued MapMessage subclass (with overloaded
>>>>> put
>>>>>>>> methods)
>>>>>>>> sounds reasonable to me given at least two of us have run into this!
>>>>>>>>
>>>>>>>> -Carter
>>>>>>>>
>>>>>>>> On Wed, Mar 24, 2021, at 19:29, Remko Popma wrote:
>>>>>>>>> I called it StringMap because the keys must be Strings. Admittedly
>>>>> not a
>>>>>>>>> great name. :-)
>>>>>>>>>
>>>>>>>>> Not sure exactly, but there may be cases where this change could
>>>>> cause an
>>>>>>>>> issue:
>>>>>>>>> putAll(final Map<String, String> map) ->
>>>>>>>>> putAll(final Map<String, Object> map)
>>>>>>>>>
>>>>>>>>> On Thu, Mar 25, 2021 at 2:12 AM Ralph Goers <
>>>>> ralph.go...@dslextreme.com
>>>>>>>> <mailto:ralph.goers%40dslextreme.com>>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>> I looked the other day and wondered the same thing Volkan did.
>>>> There
>>>>>>>> are
>>>>>>>>>> no unit tests and the contributor didn’t even indicate that he had
>>>>>>>> tried
>>>>>>>>>> it.
>>>>>>>>>>
>>>>>>>>>> I was initially concerned that the underlying Map wouldn’t support
>>>> it
>>>>>>>>>> since it has StringMap in its name. It turns out the values are
>>>>>>>> objects.
>>>>>>>>>>
>>>>>>>>>> Technically I don’t think this would break compatibility. Any code
>>>>>>>>>> referencing the put(String, String) would automatically map to
>>>>>>>> put(String,
>>>>>>>>>> Object). He didn’t modify the get method which would have broken
>>>>>>>>>> compatibility.
>>>>>>>>>>
>>>>>>>>>> Ralph
>>>>>>>>>>
>>>>>>>>>>> On Mar 24, 2021, at 8:27 AM, Matt Sicker <boa...@gmail.com
>>>> <mailto:
>>>>>>>> boards%40gmail.com>> wrote:
>>>>>>>>>>>
>>>>>>>>>>> Pretty sure that would break binary compatibility since it
>> removes
>>>>>>>> the
>>>>>>>>>>> String method. I think it might be addable but not removed like
>>>>> that.
>>>>>>>>>>>
>>>>>>>>>>> On Wed, 24 Mar 2021 at 02:39, Volkan Yazıcı <
>>>>> volkan.yaz...@gmail.com
>>>>>>>> <mailto:volkan.yazici%40gmail.com>>
>>>>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> Hello,
>>>>>>>>>>>>
>>>>>>>>>>>> Adding non-String-typed value support to MapMessage was also
>>>>>>>> something
>>>>>>>>>> on
>>>>>>>>>>>> my radar too. But this PR replacing String with Object in two
>>>> lines
>>>>>>>>>> seems
>>>>>>>>>>>> too good to be true to me. Does anybody mind taking a second
>> look
>>>>> at
>>>>>>>>>> this,
>>>>>>>>>>>> please?
>>>>>>>>>>>>
>>>>>>>>>>>> Kind regards.
>>>>>>>>>>>>
>>>>>>>>>>>> ---------- Forwarded message ---------
>>>>>>>>>>>> From: Henry Widd <notificati...@github.com <mailto:
>>>>>>>> notifications%40github.com>>
>>>>>>>>>>>> Date: Tue, Mar 23, 2021 at 4:58 PM
>>>>>>>>>>>> Subject: [apache/logging-log4j2] MapMessage put methods should
>>>> not
>>>>>>>>>> mandate
>>>>>>>>>>>> String values (#477)
>>>>>>>>>>>> To: apache/logging-log4j2 <logging-log...@noreply.github.com
>>>>>>>> <mailto:logging-log4j2%40noreply.github.com>>
>>>>>>>>>>>> Cc: Subscribed <subscri...@noreply.github.com <mailto:
>>>>>>>> subscribed%40noreply.github.com>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> the underlying Map is typed <String,Object> so the put methods
>> on
>>>>>>>>>>>> MapMessage can also be.
>>>>>>>>>>>> ------------------------------
>>>>>>>>>>>> You can view, comment on, or merge this pull request online at:
>>>>>>>>>>>>
>>>>>>>>>>>> https://github.com/apache/logging-log4j2/pull/477
>>>>>>>>>>>> Commit Summary
>>>>>>>>>>>>
>>>>>>>>>>>> - MapMessage put methods should not mandate String values
>>>>>>>>>>>>
>>>>>>>>>>>> File Changes
>>>>>>>>>>>>
>>>>>>>>>>>> - *M*
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>
>> log4j-api/src/main/java/org/apache/logging/log4j/message/MapMessage.java
>>>>>>>>>>>> <
>>>>>>>>>>
>>>>>>>>
>>>>>
>>>>
>> https://github.com/apache/logging-log4j2/pull/477/files#diff-f03ffe9ceefd37c87fd118ce591bd8ad288e43b08cd663dde14441f4e7c117ef
>>>>>>>>>>>
>>>>>>>>>>>> (6)
>>>>>>>>>>>>
>>>>>>>>>>>> Patch Links:
>>>>>>>>>>>>
>>>>>>>>>>>> - https://github.com/apache/logging-log4j2/pull/477.patch
>>>>>>>>>>>> - https://github.com/apache/logging-log4j2/pull/477.diff
>>>>>>>>>>>>
>>>>>>>>>>>> —
>>>>>>>>>>>> You are receiving this because you are subscribed to this
>> thread.
>>>>>>>>>>>> Reply to this email directly, view it on GitHub
>>>>>>>>>>>> <https://github.com/apache/logging-log4j2/pull/477>, or
>>>>> unsubscribe
>>>>>>>>>>>> <
>>>>>>>>>>
>>>>>>>>
>>>>>
>>>>
>> https://github.com/notifications/unsubscribe-auth/AAARTSKGBRHC4NG637EHA4LTFC3BTANCNFSM4ZVO7L2Q
>>>>>>>>>>>
>>>>>>>>>>>> .
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>
>>
>>
