Re: [Jsdsi-devel] CertStore Branch

[Sean R. <sra...@ae...>]

Sameer Ajmani wrote:

>These changes look good.  A few notes:
>- Provide some package-level documentation -- what's a DAO?  (Data
>Access Object)  What are the important interfaces of this package? 
>(CertificateDAO)  What implementations are provided? (Jdbc)
>- You should extract the Multimap-based implementation of
>jsdsi.CertStore into a DAO, "LocalCertificateDAO", and make
>jsdsi.CertStore a subclass of certstore.JsdsiCertStore that uses
>LocalCertificateDAO. (the following bullets clarify how to go about
>this)
>- AbstractJsdsiCertStore should implement
>init(CollectionCertStoreParameters params) by calling
>this.addCertificate(c) for eact Certificate c in
>params.getCollection().iterator().  addCertificate is a new abstract
>method of AbstractJsdsiCertStore.
>- JsdsiCertStore implements addCertificate(c) by calling dao.store(c).
>- LocalCertifificateDAO implements store(c) by populating its
>MultiMaps as shown in jsdsi.CertStore.init().
>- AbstractJsdsiCertStore.getCertificates: change "( X instanceof Y) ==
>false" to "!(X instanceof Y)"
>- There's a misspelled file: jsdsi.JsdiRuntimeException (there's also
>the correctly-spelled version)
>- I personally don't like RuntimeExceptions -- I'd rather the compiler
>tell me that there's an exception on the way, even if all I can do is
>propagate it.
>- I thnk JsdsiCertStoreException ought to be a checked (non-runtime) exception.
>- JdbcCertificateDAO.store() is hard to follow.  Instead, define these methods:
>byte[] getIssuer(Certificate)
>byte[] getSubject(Certificate)
>byte[] getCompatible(Certificate)
>byte[] getLocalName(Certificate)
>byte[] getName(Certificate)
>- and populate the "params" array with calls to these methods
>In JdbcCertificateDAO.retrieve(), extract the long if-else chain into
>a method that does:
>return retrieve.(...);
>- in each if-branch.  This makes the code easier to read, since it's
>clear "result" is only assigned to once in retrieve().
>
>Overall, looks cool.  Does the prover work correctly with the
>JdbcCertStore? (it should!)  Is JdbcCertStore a distributed store?
>
>Thanks for the contribution.  Let me know if you want me to look at
>revisions, and assuming all is well, we can merge it in to the
>mainline.
>
>Sameer
>
>On Tue, 23 Nov 2004 14:32:08 +0000, Sean Radford
><sra...@ae...> wrote:
>  
>
Sameer,

Cheers for your input. Hopefully, I'll be able to work on it this w/e.

Sean

-- 
Dr. Sean Radford, MBBS, MSc
sra...@ae...
http://www.aegeus-technology.com/