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
>>>>>>>>>
>>>>>>>>>> .
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>>>
>>>
>>
