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