On 14 August 2011 22:38, Gary Gregory <garydgreg...@gmail.com> wrote: > Hi, > > On Aug 14, 2011, at 10:19, sebb <seb...@gmail.com> wrote: > >> On 14 August 2011 03:02, sebb <seb...@gmail.com> wrote: >>> On 12 August 2011 20:56, Gary Gregory <garydgreg...@gmail.com> wrote: >>>> Hello All: >>>> >>>> For a first cut at generics support in Codec, please checkout the >>>> branch >>>> https://svn.apache.org/repos/asf/commons/proper/codec/branches/generics >>>> >>>> I wrote a migration guide in the root of the project called >>>> Codec2-Migration.htm. >>>> >>>> Let's discuss. >>> >>> The original code used Encoder and Decoder interfaces which operated >>> on Object, and extended these with >>> BinaryEncoder (byte[]) >>> StringEncoder(String) >>> >>> So for example StringEncoder classes need to implement >>> encode(Object) : Object >>> encode(String) : String >>> >>> As far as I can tell, those interfaces cannot be directly generified, >>> because type erasure causes a method signature clash. >>> So adding generics here means breaking binary compatibilty. >>> >>> Question is, what does adding generics to these interfaces actually provide? >>> Would it not be just as effective to deprecate Decoder and Encoder, >>> and code can then use either BinaryEncoder or StringEncoder (etc)? >>> >>> At the moment test code is of the form: >>> >>> Encoder enc = new Base64(); >>> ... >>> byte [] ba = (byte[]) enc.encode(binary)); // unchecked cast >>> >>> However this could be written: >>> >>> BinaryEncoder enc = new Base64(); >>> ... >>> byte [] ba = enc.encode(binary)); // no cast needed >>> >> >> Note that the Encoder/Decoder interfaces do not _require_ generification. >> > > Well sure. Strictly speaking that's true but I still like the branch > code better. > >> The only non-generified part of trunk is StringEncoderComparator. >> > > I do not quite look at it that way, since I created the branch ;) if > you just want to fix warnings then yes that class needs patching. But > my claim in the branch is that the component is better and type-safe > with generics. > >> This appears from the name as if it compares Strings, but in fact it >> compares Objects in the current code. >> Making it implement Comparator<Object> fixes the warning without >> compromising binary (or source?) compatibility. >> >> The StringEncoders generate an EncoderException if the parameter is >> not a String. >> This does mean that comparison with a non-String will return 0 (equal) >> which is rather strange, but that is the way that the code current >> works. > > That sure sounds like a bug.
Well, the method .StringEncoderComparator.compare(Object, Object) Javadoc says: @return the Comparable.compareTo() return code or 0 if an encoding error was caught. Does seem like a design bug, as one cannot distinguish failure from equality. >> >> So what I suggest is - let's release Codec as a binary compatible >> implementation, at least for now. > > ATM there incompat changes due to deprecations and other bits. Yes, but those could be reverted if necessary. >> >> If we do decide to break binary compatibility in a later release, then >> I think the Encoder/Decoder hierarchy needs a bit more work. >> For example, several of the new xxxCodec classes only implement >> Decoder, not Encoder; this is because of the need to avoid erasure >> clashes. > > Another set of refinements the ;) I don't think the redesign goes quite far enough to fix the interface inheritance problem. What I'm suggesting is to take smaller steps to achieve the goal. Release a binary-compatible version now - which has lots of other useful fixes - and release a completely new version later. If problems are found with the new stuff, any API changes can be rolled into the next major version. >> >> I just wonder if trying to use generics here is not causing more >> problems than it solves? >> It's not as if there are lots of different types that are used here - >> only String, byte[] and char[]. >> Perhaps we could just add CharEncoder/CharDecoder interfaces instead? > > Please see my proposal for the typing of the new BM encoder which I > want to type as Set<String> encode(String) in a previous email. > > Thank you, > Gary > >> >>>> I plan on not touching trunk until the generics code is flushed out >>>> and brought back to trunk. >>>> >>>> Thank you, >>>> Gary >>>> >>>> -- >>>> http://garygregory.wordpress.com/ >>>> http://garygregory.com/ >>>> http://people.apache.org/~ggregory/ >>>> http://twitter.com/GaryGregory >>>> >>>> --------------------------------------------------------------------- >>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>>> For additional commands, e-mail: dev-h...@commons.apache.org >>>> >>>> >>> >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >> For additional commands, e-mail: dev-h...@commons.apache.org >> > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
