On Fri, 2006-01-13 at 09:32, David M Johnson wrote: > On Jan 13, 2006, at 11:44 AM, Allen Gilliland wrote: > > > Okay, I am going to gripe about this one a bit. I think it's > > rather stupid that xdoclet is telling us how to write our code. > > There is no reason why I shouldn't be able to extend a servlet > > class and have both parent and child mapped as servlets. The > > reason I made this change is because I would like to tidy up the > > velocity servlets and rename the BasePageServlet to just > > PageServlet, which makes more sense. > > That's a valid complaint. I'm pretty sure that the XDoclet folks > would consider this to be a bug.
true, but that's not likely to happen any time soon. > > Why not do it like this (or something similar): > > PageServlet - no servlet mapping > WeblogServlet - /page/* > PreviewSevlet - /preview/* > RssServlet - /rss/* > AtomServlet - /atom/* that's the same thing we have now. BasePageServlet - no mapping PageServlet - /page/* (yet there is no code in this class) PreviewServlet - /preview/* rss and atom are not actually servlets, they are just redundant mappings to the FlavorServlet. I wanted to fix it because that doesn't make any sense. There is no real reason to have BasePageServlet as an abstract class and have PageServlet extend it. The current BasePageServlet *is* the PageServlet in reality, so the current object hierarchy is a bit broken and misleading. It should be .. PageServlet - /page/* PreviewServlet - /preview/* since the preview servlet is a true extension of the functionality of the PageServlet. I think little inaccuracies like this pile up and make the code less concise and ultimately harder to maintain, and that matters quite a bit to me. We already have xdoclet inserting servlet mappings from servlets.xml and servlet-mappings.xml file in the metadata/xdoclet directory. I would prefer to correct this and put the servlet mappings in there rather than leave things the way they are now. -- Allen > > etc. > > > - Dave >
