2011/1/26 Oleg Kalnichevski <[email protected]>: > [...] > Fine. The problem is that Service Locator is considered an anti pattern > by many, an opinion I fully subscribe to.
I don't have a strong opinion on this. IMHO it depends on the context (also, an antipattern is usually compared to a better alternative to reach the same goal). If you want to remove it from svn feel free to do that. I just care about the operational fixes/improvements I did there and don't care about the locator or the dom api (sooner or later it would be good to have this api, but it is not an high priority IMO). Consider it a test. I did it in trunk simply because no one else was working there and we usually prefer coding to happen in trunk (in fact the locator+factories is something that took few hours, nothing more than an experiment and we probably wasting too much time "discussing" it when it can either be improved or replaced by alternatives) I'm not against removing the locator, removing the "abstract classes style" and so on (and you should feel free doing this, really). The main "design" thing I care is about package dependencies: I don't want dependency cycles between packages (I know some developers don't care about this, but in my experience it is very important and I try to defend this as I can: I'm aware people think I'm extremist about this). > [...] > What is wrong with plain old java classes? What is it _exactly_ that was > wrong with Message classes in 0.6? goals in my mind when I did the change: 1) having a single package exposing dom functionality/classes so client classes simply have "import mime4j.dom.*;" and not some import from parser some from stream some from message and some from field. 2) make sure this package had no direct dependencies on the parsing package (DOM should not depend on a parsing implementation: DOM is about structure, not parsing) 3) allow future release to be able to add new methods/behaviours without breaking compatibilty. I remember 0.6 had many other issue I had to refactor in order to come up with clean dependency tree and fix some bug regarding consistency (headless parsing, header folding, empty lines), but if you want to backport fixes and keep the "Message" style from 0.6 I can live with it too. I'm sorry but my memory don't let me remember all of the issue I saw in 0.6 (I don't even remember what I ate yesterday). Another motivation was using an API java developers were used to (Most java developers used/understand xml api so it was a good match). > I am not sure. Now we have a situation when neither you nor I like the > existing DOM API that much. If I understand the way you liked the 0.6 version then it should be simple to make you like trunk again. 1) Just leave interfaces in dom. 2) Remove the factories and service locator from dom (or ignore they exists, they are just something "additional" you are not forced to use them). 3) Tell people to instantiate Message implementation from message package (MimeMessage / MessageImpl). How is so much different from 0.6 with dom interfaces extracted to another package? Are you instead saying you don't want the "dom" package at all? While I try to remember something else from the upgrade of my "client code" from 0.6 to 0.7-SNAPSHOT I personally prefer the way some configuration is exposed via the new factories instead of the 0.6 method of passing a MimeEntityConfig whenever you wanted to alter the default behaviour (I think config objects like this are unexpected from a java library user). jDKIM with mime4j 0.6: http://svn.apache.org/viewvc/james/jdkim/trunk/main/src/main/java/org/apache/james/jdkim/impl/Message.java?revision=824254&view=markup You can see I had to extend MimeTokenStream just to be able to configure a new max line length. Also I had to use the low level token stream for a simple task like parsing message headers. jDKIM with mime4j 0.7-SNAP (didn't update to the current trunk, it is still against trunk before you worked on it): http://svn.apache.org/viewvc/james/jdkim/trunk/main/src/main/java/org/apache/james/jdkim/impl/Message.java?revision=998320&view=markup It still uses MimeEntityConfig but you can see the comment where I was planning to have similar configurations exposed as factory properties: -- MessageBuilderFactory mbf = MessageBuilderFactory.newInstance(); 68 mbf.setAttribute("MimeEntityConfig", mec); 69 // mbf.setProperty("MaxLineLength", 10000); --- Once MaxLineLength was a factory configuration property the jDKIM would be using only dom classes (and not message/parser/stream lower level packages). Also the code moved from 184 to 130 LOC. Unfortunately my experience with mime4j <=0.6 is limited to jdkim because the only other project where I used mime4j used 0.7-SNAP since the first version because I use the validation and the Monitor to report errors found while parsing (not available in 0.6). > Anyway, I'll stick to my original plan, bring mime4j to a state in which > it could be released as 0.7 and step back again. OK (and feel free to drop my trunk tests if you have ideas, really) Stefano PS: This long messages are not to "defend" my opinion, but just to explain trunk code and historical steps.
