Yes, if you do that then also including getObject makes sense.
Ralph
> On Mar 25, 2021, at 1:56 PM, Tim Perry <tim.v...@gmail.com> wrote:
>
> Oh, I see. I meant to have it return an object. I should have written:
> Object getObject(String key)
>
> I was meaning to ask: if we allow putting Object instead of just String
> into the map, then shouldn't we provide a way to get the object out?
>
> Sorry about my poor proofreading.
>
>
> On Thu, Mar 25, 2021 at 12:17 PM Ralph Goers <ralph.go...@dslextreme.com>
> wrote:
>
>> 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
>>>>>>>>>>>>>
>>>>>>>>>>>>>> .
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>>>
>>>>
>>
>>
>>
