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 Tue, 08 Feb 2011 10:58:01 GMT
2011/1/31 Oleg Kalnichevski <olegk@apache.org>:
> On Mon, 2011-01-31 at 01:09 +0100, Stefano Bagnara wrote:
> ...
>> >> 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?
> In all honesty, I do not. I'll be more than happy to make ParseParams
> non nullable, if that matters to you.

I took some time to better think about it.
I'm convinced that parse(InputStream,ParseParams,DecodeMonitor)
doesn't make sense.
My main explanation is that I have just 2 use cases and:
1) uses ParseParams but doesn't use DecodeMonitor
2) uses DecodeMonitor but doesn't use ParseParams.

I also think they are common use cases. Just thinking about it:
- you use DecodeMonitor to be informed of malformations or to force
strict mode parsing (for validation) or to be informed for bad
- you instead use ParseParams when you don't want to recurse into
parts or you don't want to decode.
So I see that most users will need one or the other but rarely needs
both at the same time.

Is there a way (that you would accept) we can pass ParseParams and
DecodeMonitor to the Builder or (even better) to the Factory so that
we can still use parse(InputStream)?

PS: another option for the "content decoding" feature would be to
expose a "getOriginalInputStream" aside the "getInputStream" so that
you don't have to configure the parsing but instead your "client code"
uses the correct method depending on what it expects to deal with.
Maybe this is simpler to understand/use for users (maybe we have some
technical implication, but I'm willing to have a look at this if you
like the idea), WDYT?

>> >> 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();
>>     ....
>>   }
>> }
> It is not something I propose. This is the way mime4j always worked with
> structured headers.
> Let me repeat my question. Do you think #getDispositionType()
> #getFilename() methods represent inherent properties of a MIME entity
> that apply to all its instances? If so, those methods should be added to
> the Entity interface. If not, having to use the lower level API to
> access structured fields is the price to pay for not polluting the
> higher level API with methods relevant only for a subset of implementing
> classes.

Yes, we are "mime4j" so we should expose mime stuff to make it easier
to deal with mime.
It is part of an old and estabished standard tracked rfc (rfc2183).
My main concern is with the API forcing the user to use class casting
to extract a "basic" information about the mime content.
I'm not sure I understand why you removed it during your refactoring:
was it because of "technical issues" or because didn't like to expose
that stuff?

>> 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.
> I use mime4j for private projects, mostly low level 'core' code, though.

Do you use the dom module or just the core module?
In the first case, do you use full "structured"/"decoded" parsing or
other features like headless parsing, flat parsing, non-decoded
contents and so on?

> Bringing mime4j back to HC is an appealing option, but not sooner mime4j
> goes 1.0 GA.

Thank you for the explanations,

PS: if you prefer to release 0.7 as is and move every of my
"objections" to a next release just go ahead (I hoped someone else to
join the conversation to deal with our different styles so to have a
simpler majority based consensus, but we can't have everything we hope
:-) ).

View raw message