Stefano Bagnara wrote:
2010/1/8 Oleg Kalnichevski <[email protected]>:
Stefano Bagnara wrote:
2010/1/8 Oleg Kalnichevski <[email protected]>:
With so many classes moved to different packages an iterative merge
would just be too hard. I am +1 to merging the entire branch down to
trunk. Remaining issues can be dealt with once the branch has been
merged.
Minor stuff:
(1) I also would like to propose a few minor changes / renames. Ideally,
I would like the 'steam' package to be fully usable out of the box. So,
it would be good if DefaultBodyDescriptor was moved to 'steam' and
renamed to BasicBodyDescriptor for consistency. I also think
FullBodyDescriptor is a better name for MaximalBodyDescriptor
DefaultBodyDescriptor would reintroduce the field parsing dependency
to the stream package.
Unless I am missing something, I can't see any mime4j dependencies other
than on 'util' and 'stream'
---
import org.apache.james.mime4j.stream.BodyDescriptor;
import org.apache.james.mime4j.stream.MutableBodyDescriptor;
import org.apache.james.mime4j.stream.RawField;
import org.apache.james.mime4j.util.MimeUtil;
---
I do not see why this class cannot be moved to 'stream' without introducing
cyclic dependencies.
I'll check it. In my mind it was dependin on field parsing, but I
remember I moved there the "getHeaderParams" method, so it does its
own parsing. In my mind the right solution was to move also that
method to the field parsing packages and so also this class would
depend on the field parsing, btw maybe you are right and we can move
the default body descriptor there, by now.
So, unless we say that we don't care about
separation between stream parser and field parsing, there is no way to
use stream alone.
I just do not want to end up in a situation when 'stream' package simply
cannot be used without 'parser' stuff thus rendering the whole idea of
separating the two pointless.
It is a detail, but I don't agree with this reasoning. packages help
defining the project structure and the component dependencies.
Whenever it is possible to put evidence on the dependencies of a group
of classes and their concerns it is a good practice to do that. With
your reasoning abstract packages (interfaces and abstract classes
only) wouldn't exists because they cannot be used alone!
There is nothing that makes 'stream' package abstract.
BasicMimeTokenStream is a concrete class and therefore I think 'stream'
package should have a concrete MutableBodyDescriptor implementation
At the same time I think ContentHandler and AbstractContentHandler
should go to 'parser' package. They are not used anywhere in 'stream'
and even conceptually do not belong there. They belong to 'parser'.
Oleg
