wpg-proxy-development

[Wpg-proxy-development] code review

[Alan M. <a_j...@ya...>]

John,
   
  I am working my way through the code but will probably have a few questions for you.  For now I can see the following immediate questions:
   
  * 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.
   
  * 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.
   
  * 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.
   
  * 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 ?
   
  all the best,
  Alan.


---------------------------------------------------
You can find my GPG key (a_j...@ya...) at
ETH PGP Keyserver: http://www.tik.ee.ethz.ch/~pgp/
---------------------------------------------------
 		
---------------------------------
Der neue Internet Explorer 7 in deutscher Ausführung ist da!

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.

Re: [Wpg-proxy-development] code review

[John S. <jo...@we...>]

Guys,

That sounds good please go ahead and apply those changes, I am traveling
until the 25th and my linux development laptop just lost it=92s hard =
drive, so
I am a little disconnected. :-(

I can still ssh into my main system, but it is a slightly less =
accessible
server.

Thanks, John

=20

  _____ =20

From: wpg...@li...
[mailto:wpg...@li...] On Behalf =
Of
Juan Gonz=E1lez
Sent: Tuesday, January 16, 2007 11:36 AM
To: wpg...@li...
Subject: Re: [Wpg-proxy-development] code review

=20

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.=20


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).=20

=20

* 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,
?):=20

Java HTTP Proxy Library (wpg-proxy),=20


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

=20

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=20

=20

* 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 ?=20


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.=20

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...=20

all the best,

Alan.


=20

=20

same,
Juan.

Re: [Wpg-proxy-development] code review

[Alan M. <a_j...@ya...>]

All,
   
  ..my last email seems to have got swallowed by the SMTP god so here is another attempt!
   
  I'd like to tackle Proxy as I have a few ideas here on refactoring that I can get done in the coming days (incl. how to fit in the SSL etc.) 
   
  Juan, feel free to work away on the other things (e.g., license, formatting, enums etc.) and we can check again next week where we stand on everything - is that ok with you ?
   
  Regards,
A.

John Southerland <jo...@we...> schrieb:
        v\:* {behavior:url(#default#VML);}  o\:* {behavior:url(#default#VML);}  w\:* {behavior:url(#default#VML);}  .shape {behavior:url(#default#VML);}        st1\:*{behavior:url(#default#ieooui) }                Guys,
  That sounds good please go ahead and apply those changes, I am traveling until the 25th and my linux development laptop just lost it’s hard drive, so I am a little disconnected. L
  I can still ssh into my main system, but it is a slightly less accessible server.
  Thanks, John
     
    
---------------------------------
  
  From: wpg...@li... [mailto:wpg...@li...] On Behalf Of Juan González
Sent: Tuesday, January 16, 2007 11:36 AM
To: wpg...@li...
Subject: Re: [Wpg-proxy-development] code review

   
  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.



-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV_______________________________________________
Wpg-proxy-development mailing list
Wpg...@li...
https://lists.sourceforge.net/lists/listinfo/wpg-proxy-development



---------------------------------------------------
You can find my GPG key (a_j...@ya...) at
ETH PGP Keyserver: http://www.tik.ee.ethz.ch/~pgp/
---------------------------------------------------
 		
---------------------------------
NEU: Fragen stellen - Wissen, Meinungen und Erfahrungen teilen. Jetzt auf Yahoo! Clever.

Re: [Wpg-proxy-development] code review

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

Great, i'll work on that this week and we'll discuss back next week.

See you

2007/1/17, Alan Moran <a_j...@ya...>:
>
> All,
>
> ..my last email seems to have got swallowed by the SMTP god so here is
> another attempt!
>
> I'd like to tackle Proxy as I have a few ideas here on refactoring that I
> can get done in the coming days (incl. how to fit in the SSL etc.)
>
> Juan, feel free to work away on the other things (e.g., license,
> formatting, enums etc.) and we can check again next week where we stand on
> everything - is that ok with you ?
>
> Regards,
> A.
>