Stefano Bagnara wrote:
2010/1/7 Oleg Kalnichevski <[email protected]>:
All I need is to have a dependency free token stream parser. If you
can suggest a better option that extracting the BaseMimeTokenStream
abstract class I'm fine with it. While I may like introducing an
interface, I fail to see how it fixes any issue here. Can you go in
details? (I don't understand why the abstract class hurts you so
much..)
Because it is not abstract to start with and is called Basic* not Base*.
I simply do not understand the distinction between the two classes from
the functional standpoint. Why one concrete class has been thrown into a
public api package and another one into the impl package. This just does
not sound right.
If you have any better name you're welcome! I usually don't care too
much about names at this step of my refactorings. So I agree that most
class names and most package names can be fixed (I tried to change as
few as I could to help reviewing, and when I introduced new names I
chose the one that made sense to me at that moment, nothing written in
stone).
The Basic parser is a parser that know nothing about field parser
implementations, it simply contains the basic logic. You can use it if
you need your own BodyDescriptor and the 2 basic implementations are
not enough for you, or if you want to use the parser without the whole
field dependency.
About parser vs parser.impl I agree with you. parser.impl could be
renamed to parser and parser could be moved to parser.stream or
something similar. This way the entry points remain in the same
package. Also I don't like to much "impl" as a package name. I used
impl everywhere because this meant moving less code than doing
something else. E.g: I moved message implementation from message to
message.impl, but maybe a better approach can be to rename "message"
to "dom" and put back "message.impl" in "message". Same for field. my
"field" could be moved to "dom.field" and then "field.impl" moved back
to "field". I'm really open to this, it's just I thought I would have
introduced more changes to your eyes. If you think it worth doing this
move I'll do this in the branch so you can see it applied.
I will work on an alternative approach in the coming days
Cool!
(2) Please make sure LineReaderInputStream#unread() does not impose a
particular mode of operation.
This is easy to change. I still think this is worse (performance and
memory wise) than the current solution. It is a really internal class
that should not be used by users and, in the last revision, it already
protects from multiple unread calls using exceptions. We use the
"unread" method in a single place of our code (furthermore this
already exists and works, other solutions should be implemented). If
we don't want to optimize it at the extreme maybe we can switch the
whole line reading behaviour to a Buffered Reader, so that we can
simply use mark/reset. But even mark/resent impose a particular mode
of operation. The fact is that we talk about buffers. If you don't
want to impose limits then you need infinite memory :-)
See my main issue above.
Thank you :-)
Stefano
If there are no major objection, I could volunteer to work on merging
down three low level packages: utils, io, and parser from the Stefano's
branch and also try to resolve the MimeTokenStream issue.
Oleg
