james-mime4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Oleg Kalnichevski <ol...@apache.org>
Subject Re: [cycleclean] minor naming tweaks; was Re: [cycleclean] branch review and questions
Date Fri, 08 Jan 2010 18:59:42 GMT
Stefano Bagnara wrote:
> 2010/1/8 Oleg Kalnichevski <olegk@apache.org>:
>> Stefano Bagnara wrote:
>>> 2010/1/8 Oleg Kalnichevski <olegk@apache.org>:
>>>> 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'.


View raw message