Hello, Ben! Great catch!
I suggest we make the change to throw an exception in Tapestry 5.9.0 with a big warning in the release notes. Cheers! On Sat, Jul 29, 2023 at 7:55 AM Ben Weidig <b...@netzgut.net> wrote: > > Hi there! > > I was trying to add "static empty()" methods to JSONArray and JSONObject to > reduce the creation of new instances if we know it's supposed to be empty, > like Collections does with emptyList()/emptyMap() > > Adding the methods isn't the issue (creating private constructor accepting > the underlying storage, using the appropriate Collections.empty... method), > but what I discovered while writing the unit tests. > > The JSONArray method "Object remove(int index)" doesn't explode when used > with an invalid index. > The documentation states this behavior, but I think that's wrong or at > least inconsistent with the rest of the type. > There's even a JSONArrayIndexOutOfBoundsException that's used by "Object > get(int index)" and, therefore, by all other index-based methods. > > This discrepancy in expected behavior seems weird to me, and if we decide > to elevate JSONArray to implement List<Object> in the future, as it's > almost there anyway, it goes against the documented contract of List<T>. > > However, it would be a possible breaking change for people relying on the > non-explody behavior. > > Thoughts? > > > Cheers > Ben -- Thiago --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tapestry.apache.org For additional commands, e-mail: dev-h...@tapestry.apache.org
