Harmeet,
> > As discussed earlier, there are a number of RFC violating bugs. This > > was central to one of them. > > Please point out where you have discussed removing article writer >From your reply to my message: > If these bugs can be resolved without restructuring code, you should check > them in. > > If you need to rearchitect or refactor for these fixes, you should post > your > proposal and your fixes. > As I've already mentioned, you seem to put removing a class in some sort of special "design change" category. I have no idea why. I don't even know what you mean by not "restructuring code", since every change is restructuring. In accord with our discussions, I committed my changes. If you don't like it, -1 it. Personally, I'm sick of the buggy NNTP server and will be posted a vote to remove it from James shortly. > > There was an attempt in the handler to collapse the STAT, HEAD, BODY, > > and ARTICLE implementations into a single implementation. This led > > directly to item #5 in Bug #13564. > > > > ArticleWriter did not impede this. > One could have passed the response code to > private void doARTICLE(String param,ArticleWriter articleWriter) > rather than collapse and refactor This is ridiculous. Why on earth would you break abstraction like this? Aside from violating the semantic intent of the protocol - STAT, BODY, ARTICLE, and HEAD are all separate commands, there's no actual benefit. The code is further obfuscated (which is why there were a couple of hidden bugs in it) and the individual messages can't be customized without adding yet another parameter to the doARTICLE. But more critically, we're seeing the same "that's not the way it used to work" argument. It didn't work before. It works now. And it gets rid of an extra interface and factory class. It reduces the semantic weight of the code. Decouples the individual commands (those that are not aliases of one another). > > > > Moreover, the ArticleWriter was conceptually flawed. Because it > > included protocol specific items (terminating ".", what-have-you), > > wasn't really a writer of articles. > > It is nntp protocol specific so what are your concerns about being > protocol > spcific ? Specific to the protocol itself. Not specific to the server, where it is exposed. > > wasn't really a writer of articles. If it were used in a repository > > implementation, it would corrupt all the articles in a repository. > > It could write out to the client the article information. If you write to > source, sure you can corrupt. So it isn't an ArticleWriter. It doesn't write articles. If I pass it a FileWriter wrapped in a PrintWriter, it won't work. > > Moreover, it contained no logic other than whether these protocol > > specific items should be written - the remainder of the article writer > > logic is internal to the NNTPArticle implementation. > > That was the intent. Well why? You introduced a new class to no effect. --Peter -- To unsubscribe, e-mail: <mailto:james-dev-unsubscribe@;jakarta.apache.org> For additional commands, e-mail: <mailto:james-dev-help@;jakarta.apache.org>
