directory-kerby mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Emmanuel Lécharny <elecha...@gmail.com>
Subject Re: [kerby] ASN1 Quick review
Date Thu, 24 Dec 2015 09:02:37 GMT
Le 24/12/15 01:56, Zheng, Kai a écrit :
> Nice picture!! My comments are embedded marked as [Kai].
>
> -----Original Message-----
> From: Emmanuel Lécharny [mailto:elecharny@gmail.com] 
> Sent: Thursday, December 24, 2015 1:29 AM
> To: Apache Directory Developers List <dev@directory.apache.org>
> 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?

Of course !

Find the attached graphml file (the source from which I generated the
picture, using yEd - https://www.yworks.com/products/yed)

>
> - 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. 
Separation of concern is actually better done with Interfaces.

> Asn1Encodable is responsible for all the encoding/decoding dirty works; and AbstractAsn1Type
would wrap a Java type value using Java generic. 
The real question here is more or less an architctural question. Let's
consider that an object might have to support multiple 'facets' (as in
http://i.cs.hku.hk/~clwang/projects/facet.html)
- should we let a Class implement a 'facet' ?
- or should we delegate this facet to an helper class ?

For encodable, I would rather think that the help class approach would
be way too complex, as each ASN1 class has its own implementation of
some of teh Asn1Encodable classes. This make me think that the first
approach is better.

Now, as all the Asn1 classes that extend AbstractAsn1Type already have
their own implementations of methods like encodingLength(), I'd rather
see the AbstractAsn1Type abstract class implement what is currently in
the Asn1Encodable class, and make Asn1Encodable an Interface (becaise
the other branch of Asn1Object aren't implementing it).

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

DO you have some types in mind ? Or is this just a door you want to
maintain open, just in case ?

> I wish the library could evolve further so be able to standalone as a complete solution
some day.
Sure, but I think it's preferable to have a clear vision and design at
each step, which can evolve in the future.

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

Here, that makes more sense.
>
> - 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. 
Forget what I suggested. It's clear that we *need* this interface, it
forces the classes to implement the methods, otehrwise we might forget
doing so, ending with an incomplete 'dump' feature.

Asn1Specifix (which name should probably be Asn1Specifics) should
implement this interface, btw.

Thanks !



Mime
View raw message