Nice picture!! My comments are embedded marked as [Kai]. -----Original Message----- From: Emmanuel Lécharny [mailto:[email protected]] Sent: Thursday, December 24, 2015 1:29 AM To: Apache Directory Developers List <[email protected]> Subject: Re: [kerby] ASN1 Quick review
Some more questions, now that I have drawn the full Asn1 hierarchy (http://pasteboard.co/fGVRQFm.png) [Kai] Thanks a lot. Very cool! Could we integrate it in the library doc? - can't we merge the Asn1Encodable and AbstractAsn1Type classes ? [Kai] I guess we can, actually before AbstractAsn1Type is very large. I would prefer to separate them becaue the roles of them are clearly different. Asn1Encodable is responsible for all the encoding/decoding dirty works; and AbstractAsn1Type would wrap a Java type value using Java generic. For now all Asn1Type object are AbstractAsn1Type, but may be not in the future, because there are possible situations where some types don't want to start with AbstractAsn1Type using the generic things. I wish the library could evolve further so be able to standalone as a complete solution some day. - same question for the Asn1Cnstructed and Asn1Collection classes [Kai] Again they were together before but separated out recently. Asn1Collection is purely for set/setoff/sequence/sequenceof. Asn1Constructed can be another case where primitive type but using constructed encoding. You could check the places where it's used. An example in Asn1Converter. - I'm not sure the Asn1Dumpable interface makes a lot of sense. Why don't we simply implement a toString() method that takes an identation as a parameter ? [Kai] Right initially when implementing the dump support we went the way, we used a separate method toStr() but found it doesn't work nicely. toString() with a indent parameter looks anti-intuitive I guess. A dumpable can be passed a dumper as well and using the dumper it can share the same underlying String builder along with all kinds of utility functions. Note only complex and bridge types like Asn1CollecionType, Asn1Any and Asn1Container need to go the way, simple types use Java toString() naturally. I thought the dumping support is very important because without it, many time it's very hard, time-consuming, and frustrating when digging into a large hierarchy structure or more than 1k bytes for a bug. It's surely a place we can further improve. Regards, Kai
