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