On Thu, 2010-01-07 at 15:32 +0100, Stefano Bagnara wrote: > Sounds like a bikeshed issue. Of all big changes what you don't like > are the 2 smaller changes :-) >
My main issue is very simple: your changes were driven primarily by API purism rather any practical benefit. Therefore I find it strange that some very subjective API issues are fixed by introducing different API ugliness such as modal API or abuse of class inheritance. There is really a lot of good stuff in your branch, which I would love to see merged to the trunk, but certain changes to the low level parser classes are questionable, and I care mostly about low level mime4j functionality. > >From your previous message where you talked about the fact that I > introduced big functional changes I thought there was much more than > this. These 2 are very minor changes that do not alter the way the > library is used by users nor any functional behaviour. > > I'm sure that if the issues are only the 2 you list here we'll find a > solution. > > more inline, > > 2010/1/7 Oleg Kalnichevski <[email protected]>: > > (1) Please revise / redo BasicMimeTokenStream / MimeTokenStream split or > > revert 895241. Consider making MimeTokenStream an interface as an > > option. > > 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. I will work on an alternative approach in the coming days > > > (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. > Stefano >
