Re: [Wpg-proxy-development] code review

[<ju...@ag...>]

Hi Alan,
   i'll add somethings to your thoughts:

* the code is a bit hard to read in many places owing to formatting - would
> you have any objection if I ran the classes through GNU indent or similar ?
> If necessary we can add a svn hook to do that sort of thing automatically.
>

I agree and I'm passing the code through netbeans indentation system and
will upload it in a little while, also I think using 1,5 generics to get
typesafe collections and use the new for loop to help making the code easier
to read (and more robust).

* to make the licensing a bit clear I'd suggest that you place a license in
> the root directory and add a template to each Java file (e.g., with a
> comment, distributed as per shipped license or similar) - I see you have
> gone for LGPL so we should adopt those conventions perhaps.
>

Just to agree on how to add this to the package, we should add something
like this to each java file (I think there is no need to name each of us,
?):

Java HTTP Proxy Library (wpg-proxy),
more info at http://wpg-proxy.sourceforge.net/

This library is free software; you can redistribute it and/or
modify it under the terms of the GNU Lesser General Public
License as published by the Free Software Foundation; either
version 2.1 of the License, or (at your option) any later version.

This library is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
Lesser General Public License for more details.

You should have received a copy of the GNU Lesser General Public
License along with this library; if not, write to the Free Software
Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA


And also add a Licence.txt file to package root with the text from
http://www.gnu.org/licenses/lgpl.txt

* a small detail perhaps but I noticed the use of protected logger members (
> e.g., in HttpMessage) - I personally feel it would be better to use
> private (static) logger members in each class since the logger name is based
> on the java package hierarchy and a logger per class makes things more
> intuitive (and easier to configure) with the logger configuration.
>

I prefer private static loggers too

* finally the classes might benefit from a mild dose of refactoring (e.g., I
> think that Proxy is a bit too big and has inner classes within where
> delegation model might be more appropriate etc.) - I could go ahead and
> model this myself without changing any functionality but don't want to make
> you too nervous about what I'm doing here - shall we discuss this in more
> detail ?
>

I would extract those classes to ease code readability and debugging, Proxy
looks a bit ugly actually. Anyway, as you said in another mail we should
work out some UML modelling to have at hand.

Enums may also find their way in (for HEADER_*, methods of requests,etc),
they might also help making the library more robust as they avoid
programming missuses from the root (like passing conect in place of connect)
and ease filter creation...

all the best,
> Alan.
>



same,
Juan.