(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

Reply via email to