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
