james-mime4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefano Bagnara <apa...@bago.org>
Subject Re: MessageBuilder refactoring; was Re: dom / message refactoring
Date Tue, 25 Jan 2011 21:09:46 GMT
2011/1/25 Oleg Kalnichevski <olegk@apache.org>:
> On Tue, 2011-01-25 at 17:49 +0100, Stefano Bagnara wrote:
>> 2011/1/25 Oleg Kalnichevski <olegk@apache.org>:
>> > ...
>> >
>> >>
>> >> >> [...] BTW I'm just loud thinking, just go ahead with
>> >> >> your plan :-)
>> >> >
>> >> > I am going to get there shortly. I just prefer to make potentially
>> >> > contentious changes in small, incremental steps, so that they are easier
>> >> > to review and agree / disagree upon.
>> >>
>> >> I understand. I'm impatient to see your changes :-)
>> >>
>> >> Happy hacking
>> >> Stefano
>> >>
>> >
>> > Stefano et al
>> >
>> > I just completed refactoring of MessageBuilder and related classes on
>> > the refactoring branch. Here is a short summary of changes:
>> >
>> > * MessageBuilder redesigned and changed to an interface
>> > * Moved #writeTo method to MessageWriter interface
>>
>> Moving "writeTo" from each "DOM node" to a MessageWriter interface
>> have the drawback that you cannot anymore serialize just part of a
>> message (just a subtree, an header, a content) because your
>> MessageWriter serializes only full messages. On the other side I see
>> the advantage of having the writer separated from the element itself
>> (reading and writing are different tasks).
>>
>
> I am sorry but that was not done by me.
>
> http://svn.apache.org/viewvc?view=revision&revision=739822

So it seems the "feature" has been removed long ago! Thanks for the link.

>> I haven't carefully looked at the code to see if you preserved some of
>> the optimizations about unparsed content streamed as-is in writeTo
>> instead of being recreated (this was good for performances and also to
>> keep input formatting when writing out a parsed message with no
>> changes).
>>
>> > * MessageBuilderFactory renamed to MessageServiceFactory and can now be
>> > used to instantiate message writers and builders. Additional factory
>> > methods can be introduced in the future.
>> >
>> > Please review and let me know if you can live with these changes.
>>
>> I hoped you wanted to deal also with DOM altering methods as I thought
>> you was not happy with dom mainly because it was an incomplete api (so
>> I probably didn't understand you well). I don't see "improvements"
>> over previous trunk
>
> My main problem was the lack of consistency in your code more than
> anything else. I never claimed I was able to create a "better" /
> "enhanced" / "more complete" API. I was merely trying to resolve
> inconsistencies while preserving the initial design and the general
> approach that you had started. If I had my way I would simply throw away
> every single class in the 'dom' packages, especially that horrid
> ServiceLoader, and reverted back to the simpler Message API that had
> existed before.

The fact is that probably ServiceLoader was required in the previous
design, using Abstract classes. That design is the only I'm aware of
that would have allowed future releases to add more methods to each
class without breaking compatibility with older sources. Now that you
removed this possibility by switching everything to interfaces I
understand that the ServiceLoader seems "horrid". Most stuff I did in
the dom package has been done with this precise goal, so I can't
"defend" that code if we remove the goal :-)  That said you also
should understand that the ServiceLoader, like most SPI model out
there, is simply an abstraction layer. You are free to directly
instantiate the implementation.

I didn't (and I currently don't) know a better way to create a forward
compatible, yet extensible, API, but I'm far from being an expert API
designer.

>> (But maybe this is mainly a matter of style /
>> different approaches to API design), but it is anyway much better than
>> 0.6 so I can live with it (power to active people).
>>
>> To avoid misunderstandings: I'm not "sad" accepting this changes, on
>> the contrary I'm happy to see someone pushing mime4j again and I just
>> try to find the time to review and explain why things were done in
>> that way.
>>
>
> How do you want me to proceed now?

I'm happy to see you pushing. *Go* *ahead* :-)  I just explained why I
did things in a given way and how I evaluate changes. This was not a
comment to stop you, but just something to let you be more informed.
As long as 0.7 will support the Monitor object, the fixed headless
parsing and the fixed field folding I will be happy (I currently have
0.7-SNAPSHOT as dependency for this and not for the DOM abstraction).

Stefano

Mime
View raw message