Hi, On Nov 17, 2010, at 3:26 PM, Bram de Kruijff wrote:
> On Wed, Nov 17, 2010 at 12:02 PM, Angelo van der Sijpt > <angelo.vandersijpt at luminis.eu> wrote: >> Hi, >> >> Good to see we have a file-based storage for our tenants now! >> >> Still, there are some things I don't really understand. >> - I noticed the storage provider can be configured using a system property; >> why not use the ConfigurationAdmin for this? > > Good point. I was writing this under the assumption I might have to > stick it under ConfigAdmin itself, but I see the Felix ConfigiAdmin > has its own filebased storage allready (and uses a Java Property to > allow overriding bundle storage). I'll change this. Okidoki. > >> - The FSTSP synchronizes all its methods; this it not necessary, as the >> TenantStorageProvider javadoc states 'The TenantStorageProvider can assume >> that synchronization will be handled externally.' > > I see... but I dont feel comfortable not synchronizing when I know my > impl of a published service cannot handle it. On the otherhand this is > an SPI so maybe I'm being a little too defensive :) In essence I'm all for defensive programming, but as you have noted, this is an SPI with a note on concurrency. By the way, we should come up with a standard pattern for these situations. > >> - It seems you use finally a little differently than I do (e.g., in >> FSTentantIdList, the m_file is final, but the m_tenantIdList is not). Our >> coding guideline does not state anything about this; should we include that? >> Something like "members should be made final if they can, and local >> variables should only be made final when they have to be". > > Seems I forgot to put the final modifier on m_tenantIdList. In general > I like making things final where possible (and sometimes relevant). No > strong opinion with regard to local variables though so +1 for your > proposal :) I will put a proposal up on the wiki. > > Regards, > Bram Angelo > > _______________________________________________ > Amdatu-developers mailing list > Amdatu-developers at amdatu.org > http://lists.amdatu.org/mailman/listinfo/amdatu-developers
