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: DOM API refactoring completed
Date Mon, 31 Jan 2011 00:09:21 GMT
2011/1/30 Oleg Kalnichevski <olegk@apache.org>:
> On Sun, 2011-01-30 at 19:33 +0100, Stefano Bagnara wrote:
>> 2011/1/28 Oleg Kalnichevski <olegk@apache.org>:
>> > Folks
>> >
>> > I did my best to improve the DOM API and to fix things what I felt were
>> > inconsistent while largely preserving the original design of Stefano.
>>
>> I tried to update jDKIM and my internal project but I see it is taking
>> a bit more than I thought, so I need some more time.
>>
>> I don't find the new method parse(InputStream, ParseParams,
>> DecodeMonitor) very much appealing:
>> 1) it uses a configuration object (ParseParams) just to pass 2
>> booleans.. not a big gain over passing the 2 booleans (unless this
>> time you used this class to support forward compatibility?).
>
> (1) Multiple boolean parameters are confusing and considered a bad
> practice.
> (2) We can easily add more parameters in the future without having to
> change the interface itself

I see you often reference Pattern/Antipatterns/Bad practice.
Don't you think that method with the "nullable" parameters had a bad smell, too?
I can see the disadvantage of method proliferation, too, but then why

>>  Also, we
>> already have a configuration object for parsing and it is
>> MimeEntityConfig.. so it seems to be confusing to have 2 configuration
>> objects (one in the "public" package and one in the "private" package,
>> but I'm not even sure you consider "message" a private package) (Also
>> maybe ParseParam.flatMode shouldn't be "true" by default).
>> 2) I'm not a fan of nullable parameters and in jDKIM I would instead
>> have to use this method using a new ParseParams and null as decode
>> monitor (the use of parsing parameters and the use of a decode monitor
>> are not related in any way, instead I this most users won''t use them
>> together: if you disable content decoding and use flat mode then you
>> don't probably expect a lot to be "DecodeMonitored").
>>
>> So, for jDKIM the latest changes are not so appealing to my eyes
>
> No problem. I'll happily revert to the 0.6 API if you like it better
> that way.

The comparison was against previous trunk and not 0.6. I already had
to upgrade jDKIM to 0.7-SNAP because of big issues with 0.6 (starting
from inability to configure the parser without subclassing it).

>>  (I'm
>> writing this mainly because I hope I completely missed your
>> "consistency" changes and there is a much better way to refactor jdkim
>> and I don't see it as I'm still "limited" on my reasonings of the
>> previous design: I know this happens, so I'm asking for some hints.).
>
> Not very surprising since you refused to see any consistency issues in
> the first place.

I never *refused* to see and the fact that I'm still making questions
is because I *always* *try* to see (so at most you can say I'm unable
to see, but not that I refuse to do that).
That's why I'm asking you to help with jDKIM upgrade so I can see how
you think mime4j should be used.
I'm always happy to learn, expecially by people that see more things
than me (and this is not ironic), but you should help people to
understand when they do not get your stuff: I was looking for hints
and not trying to surprise you.. I hope someone else that better
understand me and you is able to translate.

>> Now I moved to the other client project (a private one). In that
>> project I was using entity.getDispositionType() and
>> entity.getFilename() methods and I can't find them anymore: what is
>> the right way to access this informations now?
>
> What is wrong with ContentDispositionField? If you are convinced these
> two methods represent inherent properties of a MIME entity they should
> be added to the Entity interface

So, just to be explicit, what you propose?
Field f = entity.getHeader().getField(headername);
if (f != null) {
  if (f instanceof ContentDispositionField) {
    String fileName = ((ContentDispositionFIeld) f).getFilename();
    ....
  }
}
Or something else?

>> This project does not
>> use specific parsing configurations, but it uses a DecodeMonitor.. Do
>> you API expect I use "null" as the ParseParam in the parse call or
>> instead a "new ParseParam()" with no special change to the default
>> config?
>
> Both approaches should have the same effect.

Do you like methods promoting the use of nullable parameters?

>> > Please review and let me know if you approve or disapprove.
>>
>> I shown the jDKIM use case but I would love to consider also other use
>> cases so to be able to have a more complete understanding about how
>> the refactored api compares to the previous one.
>> IIRC you use mime4j in Http components. Have you already tried
>> upgrading HC to use the latest trunk?
>
> I had to drop the dependency on mime4j approximately 6 months ago
> because mime4j development was basically in limbo.

So, why are you still interested in mime4j? Is there another project
depending on it or anyway you plan to reintroduce mime4j in HC after
the changes?
I'm just trying to see your use case.

Stefano

Mime
View raw message