Hi Mike, Thanks for the feedback. It is really appreciated.
Can we hold off the discussion of individual points fora little while? I am really trying hard to finish the HttpServerConnection and HttpEntity related classes by the end of the weekend, so we have a feature complete implementation to work with. A lot of things will be much clearer once the server side of the story is taken into consideration Oleg Once we have more or less complete picture, I'll address On Fri, 2005-04-15 at 23:31 -0400, Michael Becke wrote: > Hi Oleg, > > I've been going over all the new 4.0 code and it looks great! I'm > getting anxious to jump in and start coding. Overall I agree with > everything you've done so far. > > Below are some comments/ideas/questions that came to mind as I was > looking over the code. Some of them are a little trivial, but I > wanted to write them down before I forgot. > > - I like the change to the HttpParams. We can probably even take it > a little further by making the Http*Params methods static and just do > things like the following: > > HttpParams params = new HttpParams(); > HttpConnectionParams.setConnectionTimeout(params, 1); > > We could then rename either HttpParams or the Http*Params classes to > differentiate them. > > - Should getInputStream() be a part of HttpEntity? Adding an > HttpIncomingEntity might be better as the InputStream is only really > necessary for the response and on the server side. > > - Header.parse() and HeaderElement.parseElements() should probably be > moved to HeaderParser. > > - Why does HttpLineParser.readRawLine() specify that the input stream > must be unchunked? > > - Is EntityConsumer just a utility class or does it have a bigger > purpose? If it's just utility we should probably move it to > o.a.h.util. > > - Why are the variables of AbstractHttpConnection transient? Do we > anticipate serializing connection? > > - To be consistent we should rename HeadersParser to HeaderParser. > > - It seems a little overkill, but to be consistent with > HttpMessage/HttpMutableMessage we should probably separate the set/get > methods in HttpEntityEnclosingMessage. > > - The third assertEquals() in TestEncodingUtils.testBytesToString() > doesn't work for me as my default charset is ASCII. I would go ahead > and change this one, but I'm not sure what you were trying to test. > > I'm going to continue going over the new code and also start filling > in some of the missing spaces. > > Nice work Oleg! > > Mike > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
