On 2009-07-26, Vincent Massol <[email protected]> wrote: > (resending since I got a failure from xwiki's mail content filter ) > > ---------- Forwarded message ---------- > From: Vincent Massol <[email protected]> > Date: Sun, Jul 26, 2009 at 11:33 AM > Subject: [Wiki Importer] Design questions > To: XWiki Developers <[email protected]> > > > Hi Arun, > > I've just started reviewing your module. I'll send questions as they > come. Here are some to start with: > > 1) Why do you need the Wiki interface? Since all implementations are > wiki-specific and since it doesn't contain any useful method I don't > see why it's needed. > 2) Why do you need 2 methods in WikiImporter interface? I would have > imagine only a single method: > > WikiImporter.import(...) > > 3) Why do you pass a byte array instead of a stream? If the import is > large a byte array will result in an OOM error. > 4) WikiImporterVelocityContextInitializer has invalid javadoc. In > addition instead of exposing each importer you could expose only a > single importer factory and pass an import syntax to it. This will > reduce the coupling and not expose the implementations to the client > side (very important). Something like: > WikiImporterFactory.createImporter(ImportSyntax). > > 5) This should never be written (in DefaultImportWikiParser & > DefaultImportWikiRenderer for ex): > > //Intialize Embeddable Component Manager > EmbeddableComponentManager ecm = new EmbeddableComponentManager(); > ecm.initialize(this.getClass().getClassLoader()); > > The component manager must alway be passed as a dependency. > > 6) You shouldn't use @Component("default") but instead @Component > 7) You should never use new > File(System.getProperty("java.io.tmpdir"). Instead you should use the > existing API located in > Container.getApplicationContext().getTemporaryDirectory(). > > I have some more comments but since they are linked to the above let's > agree first on the points above before going further. > > Thanks > -Vincent > _______________________________________________ > devs mailing list > [email protected] > http://lists.xwiki.org/mailman/listinfo/devs >
-- Sent from my mobile device _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
