Am 19.07.2011 11:29, schrieb Stefano Bagnara:
2011/7/19 Oleg Kalnichevski<[email protected]>:
On Tue, 2011-07-19 at 11:00 +0200, Stefano Bagnara wrote:
2011/7/19 Oleg Kalnichevski<[email protected]>:
On Mon, 2011-07-18 at 22:18 +0200, Stefano Bagnara wrote:
2011/7/18 Oleg Kalnichevski<[email protected]>:
On Mon, 2011-07-18 at 21:48 +0200, Stefano Bagnara wrote:
2011/7/18 Norman Maurer<[email protected]>:
Hi there,
just to be more clear. I'm not very strong with it, so if others
prefer we can just release it like it is...
I leave this to you and Oleg. Either way it is not an elegant solution
and we'll need to fix that part of the api (configuration + advanced
features) in future releases (IMO).
Stefano
Sorry for being blunt but MessageServiceFactory / ServiceLocator stuff
was not of my making. I had nothing to do with it and have nothing to
contribute there.
What we have now is the result of what I did in a first place and how
you changed it to make it pass your review.
You wanted to remove the abstract classes in favor of interfaces for
parser/builders and wanted to remove that methods I added (e.g:
setDecodeMonitor was a method of the public api abstract class), so
you can't say "I had nothing to do with it", now ;-)
Stefano
MessageServiceFactory is an abstract class, not an interface. So, I have
no idea what you are talking about. Feel free to add whatever methods to
that class you please.
http://svn.apache.org/viewvc/james/mime4j/trunk/dom/src/main/java/org/apache/james/mime4j/dom/MessageBuilder.java?revision=923012&view=markup&pathrev=1137643
You see we had an abstract MessageBuilder (now it is an interface) and
that abstract class exposed setDecodeMonitor.
http://svn.apache.org/viewvc/james/mime4j/trunk/dom/src/main/java/org/apache/james/mime4j/dom/MessageBuilder.java?r1=958717&r2=1063904&pathrev=1137643
You see this revision is when YOU changed from abstract class to
interface and removed the setDecodeMonitor. I'm talking of this very
method and that very class/interface.
I simply found that after your refactorings I was not able anymore to
use the decodemonitor (while previously I used them and used in a
typed manner), so I asked if this was intended and how you wanted me
to deal with it (I know how I would like it to work, but we already
saw that my solution is not acceptable to you, so I simply asked you
what was acceptable and it sounded weird when you replied that you had
nothing to do with this stuff).
I now added the DecodeMonitor as a setAttribute for the factory as for
your request but it was not in the factory previously.
If adding typed methods to the MessageServiceFactory (for things we
now have in setAttribute) is acceptable to you then I'll vet them and
add methods I think are to there stay for 0.8. It is an abstract class
so we can add this new methods in future with no backward
compatibility issues.
Hope this explains "what I'm talking about" :-)
Not really. What does this all have to do with the brokenness of
MessageServiceFactory / ServiceLocator stuff and the original issue
raised by Norman?
I'll try again :-)
We didn't talk about "brokenness". We talked about the fact that to
use DecodeMonitor you now have to use setAttribute using a type unsafe
method. I was explaining that I don't like too much this and he
proposed to push the attribute names as public static fields. Hope you
understand how this is related: if setDecodeMonitor for in the
MessageBuilder then we wouldn't have to use
setAttribute("DecodeMonitor", decodeMonitor) in the factory.
If you don't see the relationship it's not a problem. My mind often
does weird mental links. I record that you don't care of the
MessageServiceFactory/ServiceLocator and I'll plan a refactory "the
way I like it" for the next release.
Thank you,
Stefano
Said enough ;) Let us just release now ... I will start the VOTE thread
within the next few minutes..
Bye,
Norman