Some desing issues with XWiki

Hi all, I know everyone is really busy at this time trying to polish XWiki for the 1.0 release, so this may not be a good time to start discussing design issues. Thus feel free to ignore this message until the bug chase is over ;) Here I want to discuss about some IMO design flaws in XWiki and suggest some ways to improve them. I know the programmer's motto that goes "If it is not broken, don't fix it". While I usually agree with this, sometimes I believe some changes can improve code readeability, isolation and encourage better coding practices. Well now without further ado, some things I believe we should fix: 1) com.xpn.xwiki.XWiki has become a mess. It has become the official "put the method that does not fit elsewhere" place. Thus it is gigantic and complex. 2) there are many abstraction violations in XWiki. For example, storage. XWiki is modular, that's great, everything about storage goes in com.xpn.xwiki.store and there are nice interfaces that abstract the underlying storage for the rest of the world. Sadly this is not true, for example com.xpn.xwiki.XWiki imports XWikiHibernateStore, XWikiJcrStore, etc. So we loose the isolation. Far worse, take a look at the methods: getHibernateStore() isSQL() updateDatabase() ... I mean why are those methods in XWiki.java instead of in some store class. Those methods are encouraging bad code. Because those methods are making assumptions about the XWiki storage. And the problem with them is that they tend to promote code like 3) 3) Reinventing OO: I better explain this with an example, this is a method in XWiki.java: public List getSpaceDocsName(String spaceName, XWikiContext context) throws XWikiException { List docs = null; if (getNotCacheStore() instanceof XWikiHibernateStore) { docs = this.search("select distinct doc.name from XWikiDocument doc", new Object[][] {{ "doc.web", spaceName}}, context); } else if (getNotCacheStore() instanceof XWikiJcrStore) { docs = ((XWikiJcrStore) getNotCacheStore()).getSpaceDocsName(spaceName, context); } What are we doing here, we are doing a special case depending on the store subclass... Sadly this is done in many places in XWiki, so when I decide to add a new storage, I have to go to all of these places and add another else if statement. No way! Why not just do a dispatching call, that is what OO is for ;) Ok, so I really think we should start fixing this for XWiki 2.0. What I propose for the above points: 1) Instead of adding new methods to XWiki.java, try to fit them in a more appropriate place or even create a new class. 2) Start working on real isolation. To support old versions, mark the "bad methods" as deprecated and provide nice interfaces that do not make assumptions about the implementation. I'm happy, for storage I think Artem Melentyev is going to improve his QueryPlugin during Soc, so this will be great ;) 3) Factor the hand-made dispatchs scattered in the XWiki codebase, with dispatching. I believe this is a first step before moving towards component based architecture. I have tried to tackle some of these problems, here is a patch. Do not hesitate to criticize it, it is not supposed to be anything else than a proof of concept because I think design issues should really be discussed before doing any drastic changes. I have tested it and believe it does not break anything, yet I have a small doubt, maybe some XWiki expert can enlighten me: in xwiki.setUserDefaultGroup() a call to saveXWikiObject hibernate specific method is done. The question is is the following equivalent and store agnostic ? getDocument + addObject + saveDocument <=> getDocument + addObject + saveXWikiObject If not how are we supposed to save the Object when we are not using hibernate ? Thanks, Pablo