Hi band,

I'm having a quick look at the kerby-asn1 code, as I wanted to paly
around the idea of porting the LDAP codec to use this piece of nice
code. AFAIU, when you want to declare an object that can be encoded or
decoded, you have to extend the correct Asn1Object child. Looking at the
Ticket object, here is what I see :

...
import static org.apache.kerby.kerberos.kerb.type.ticket.Ticket.MyEnum.*;
...
public class Ticket extends KrbAppSequenceType {
    public static final int TKT_KVNO = KrbConstant.KRB_V5;
    public static final int TAG = 1;

    protected enum MyEnum implements EnumType {
        TKT_VNO,
        REALM,
        SNAME,
        ENC_PART;

        @Override
        public int getValue() {
            return ordinal();
        }

        @Override
        public String getName() {
            return name();
        }
    }

    static Asn1FieldInfo[] fieldInfos = new Asn1FieldInfo[] {
            new ExplicitField(TKT_VNO, 0, Asn1Integer.class),
            new ExplicitField(REALM, 1, KerberosString.class),
            new ExplicitField(SNAME, 2, PrincipalName.class),
            new ExplicitField(ENC_PART, 3, EncryptedData.class)
    };

I like the idea of defining the fields this way, except that I would
suggest some slight modifications :

- get read of the import ...Ticket.MyEnum.*;
- make the enum private (there is no reason we wuld like to expose it
anyway)
- name it something more meaningful, like TicketField inseatd if MyEnum
- use it with the enum name, like in new
ExplicitField(TicketField.TKT_VNO, 0, Asn1Integer.class)

<side note>
I find the "import static xxx.*;" atrocious, most of the time. It makes
it barely readable : you have to go to the top of your file to actually
*know* where the constant comes from... That may seems of for small
files, but otherwise... I accept it for Asserts, because it's really
clear, in a test context - pretty much like annotations.
</side note>

Here is what I come with :


public class Ticket extends KrbAppSequenceType {
    public static final int TKT_KVNO = KrbConstant.KRB_V5;
    public static final int TAG = 1;

    private enum TicketField implements EnumType {
        TKT_VNO,
        REALM,
        SNAME,
        ENC_PART;

        @Override
        public int getValue() {
            return ordinal();
        }

        @Override
        public String getName() {
            return name();
        }
    }

    static Asn1FieldInfo[] fieldInfos = new Asn1FieldInfo[] {
            new ExplicitField(TicketField.TKT_VNO, 0, Asn1Integer.class),
            new ExplicitField(TicketField.REALM, 1, KerberosString.class),
            new ExplicitField(TicketField.SNAME, 2, PrincipalName.class),
            new ExplicitField(TicketField.ENC_PART, 3, EncryptedData.class)
    };

(note that it's just a suggestion at this point...)

Now, let's foduc on the fields declarations. Here, we create
ExplicitFields, passing a Xxx.class as the third parameter, and a number
as the second parameter.

First of all, as the enum being used has an implicit ordering, can't we
simply avoid passing those numbers ? There is already an ExplicitField
constructor that takes only 2 parameters, deducing the number from teh
enum...

Second, why don't we write things like :


            new ExplicitField(TicketField.TKT_VNO, 0, new Asn1Integer()),

instead of passing a class ? Bottom line, the instance will be created
the same way. I don't think it will make any difference in term of
performances, and as all the object *must* extends the Asn1Object (or
implement the Asn1Type), this should be good enough.

wdyt ?

Reply via email to