2010/1/8 Stefano Bagnara <[email protected]>: > 2010/1/8 Oleg Kalnichevski <[email protected]>: >> I spent more time working with your code. The refactoring of >> MimeTokenStream makes good sense. However, the unfortunate choice of >> package and class names sent a completely wrong message. > > Happy to hear this! So, maybe I'll take the time to apply some package > name changes to let the correct message pass throught.
So, I took your "first impression" as a signal, and also prepared a refactoring with alternate package names. Today commits (they are big because moving stuff makes big diffs) include this exact change from previous cycleclean (I backupped the previous version as cycleclean-pre-packagerename). o.a.j.m.parser => o.a.j.m.stream o.a.j.m.parser.impl => o.a.j.m.parser o.a.j.m.message => o.a.j.m.dom o.a.j.m.message.impl => o.a.j.m.message o.a.j.m.field => o.a.j.m.dom.field o.a.j.m.field.address => o.a.j.m.dom.address o.a.j.m.field.datetime => o.a.j.m.dom.datetime o.a.j.m.field.impl => o.a.j.m.field So, comparing the final structure to the original trunk (not cycleclean) what I did is: 1) create a dom package, moving there abstract code and interfaces from the previous message, field, field.address, field.datetime packages, leaving in the original packages parsers and implementation specific classes. 2) merged descriptor package to the parser package. 3) create a stream package, moving there abstract code from the previous parser package (and descriptor package). stream package includes code without dependencies against the rest of mime4j (expecially the dom, field and message packages). Again, I'm not trying to force any specific structure, but only trying to show you alternative solutions by staying in the limit of better (IMO) package management (i like this one more than the the previous one). >> It led me to >> believe one package was meant to represent public API with impl package >> being implementation of that API. If the following changes can be made >> to package / class naming I can happily vote +1 to merging down the >> entire branch. Smaller issues such as #unread() method modality can be >> dealt with on the trunk. >> >> option 1) >> o.a.j.mime4j.parser -> o.a.j.mime4j.stream >> o.a.j.mime4j.parser.impl -> o.a.j.mime4j.parser >> impl.MimeTokenStream -> MaximalMimeTokenStream I know I did much more than what you suggested, but I guessed you only reviewed the parser package and that your opinion could be applied to the whole library. Let me know what you think. Stefano
