Re: [Jsdsi-devel] CertStore Branch

[Sameer A. <aj...@gm...>]

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:
> Sean Radford wrote:
> 
> 
> 
> > No sooner have I merged one branch, I've created another...
> >
> > Guys,
> >
> > I've created a branch in CVS called 'branch-certstore'.
> >
> > This is another experimental design I'd like you all to have a look at
> > refactoring the architecture of CertStore to easily allow alternate
> > stores and standard interface for also storing Certificates. Fancy
> > taking a look and getting back to me? (jsdsi.certstore package). The
> > new implementation is for JDBC.
> >
> > Regards,
> >
> > Sean
> >
> Don't suppose any of you have had a look?
> 
> --
> Dr. Sean Radford, MBBS, MSc
> sra...@ae...
> http://www.aegeus-technology.com/
> 
> -------------------------------------------------------
> SF email is sponsored by - The IT Product Guide
> Read honest & candid reviews on hundreds of IT Products from real users.
> Discover which products truly live up to the hype. Start reading now.
> http://productguide.itmanagersjournal.com/
> 
> 
> _______________________________________________
> Jsdsi-devel mailing list
> Jsd...@li...
> https://lists.sourceforge.net/lists/listinfo/jsdsi-devel
> 
> 


-- 
Sameer Ajmani
http://ajmani.net