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