Well, I've had a chance to briefly review what you've got here and for the 
most part I really like it. You've followed pretty much the same path that 
I did in my refactorings, so that's very reassuring for both of us! The 
thing that I notice though is that you have many more source files than I 
ended up with and about 4 times the number of lines of code. This isn't 
necessarily a bad thing since you've added many more comments that I did, 
but I think that maybe we should use both of these code bases as a basis 
for further refactoring.

I never ended up with full compatibility with the existing tag, but that's 
because I didn't want to add additional bulk into the tag that wasn't 
needed anymore. If your code is actually 100% compatible, then I'd 
definitely vote to start with your code base and work from there.

I would make one suggestion though, I think you should treat the HTML 
output part of your tag as just another View object and remove all the 
HTML specific stuff from the TableTag.java file.

Has anybody else had a chance to review this code yet? Thoughts?

Thanks, Fabrizio, this stuff looks great! Too bad we couldn't have come 
together sooner and saved each other some time! :)

John



On Fri, 13 Jun 2003, Fabrizio Giustina wrote:

>  
> Take a look at the new module... I initially tried to simply modify the original 
> version, but I soon realized that a complete refactoring was needed, before trying 
> to implement new features.
>  
> The new module is NEW, really different, not complete, probably with new bugs and 
> with old bugs fixed.
>  
> I'll complete it with the last missing features from version 0.8.5 in few days, then 
> we can decide if this can become the main branch or not.
>  
> fabrizio
>  
>  
> 
>       -----Original Message----- 
>       From: BOGAERT Mathias [mailto:[EMAIL PROTECTED] 
>       Sent: Fri 6/13/2003 1:27 PM 
>       To: 'Matt Raible'; BOGAERT Mathias; [EMAIL PROTECTED] 
>       Cc: 
>       Subject: RE: [displaytag-devel] display09/src - New directory
>       
>       
>       *Sigh*
>        
>       So can we all decide on which branch/tag/module to work? So we can combine our 
> efforts?
>        
>       Mathias
> 
>               -----Original Message-----
>               From: Matt Raible [mailto:[EMAIL PROTECTED] 
>               Sent: vrijdag 13 juni 2003 13:26
>               To: 'BOGAERT Mathias'; [EMAIL PROTECTED]
>               Subject: RE: [displaytag-devel] display09/src - New directory
>               
>               
>               no, you changed the "displaytag" module - this is a refactored tree by 
> Fabrizio.
>                
>               Matt
> 
>                       -----Original Message-----
>                       From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of 
> BOGAERT Mathias
>                       Sent: Friday, June 13, 2003 1:24 AM
>                       To: [EMAIL PROTECTED]
>                       Subject: RE: [displaytag-devel] display09/src - New directory
>                       
>                       
>                       So is this the version I changed yesterday?
>                        
>                       Mathias
> 
>                               -----Original Message-----
>                               From: Fabrizio Giustina [mailto:[EMAIL PROTECTED] 
>                               Sent: vrijdag 13 juni 2003 1:07
>                               To: [EMAIL PROTECTED]
>                               Cc: [EMAIL PROTECTED]
>                               Subject: RE: [displaytag-devel] display09/src - New 
> directory
>                               
>                               
>                               right, probably the name of the module was not a good 
> choice... sorry for this mistake :/ , we will chose a better name later...
>                                
>                               anyway... I've added my refactored version of the 
> code: it's not finished, there are still some things to fix from version 0.8.5 (link 
> creation with struts like attribute is missing now) and a lot of clean up to do.
>                               I also uploaded the pages for the sample website 
> (refactored using xhtml) with some documentation about changed features (highlighted 
> text). Most of the new feature/changes are still not documented anyway.
>                                
>                               Let me have any feedback about the new code. I hope to 
> complete everything in few days.
>                                
>                               fabrizio
>                                
>                               PS. I just found that in some files the @author 
> javadoc is wrong: It has been inserted automatically by my IDE, I'll fix that to 
> refer to the correct previous author
>                                
>                                
>                                
>                                
>                                
>                                
> 
>                                       -----Original Message----- 
>                                       From: Matt Raible [mailto:[EMAIL PROTECTED] 
>                                       Sent: Fri 6/13/2003 12:45 AM 
>                                       To: [EMAIL PROTECTED]; [EMAIL PROTECTED] 
>                                       Cc: 
>                                       Subject: Re: [displaytag-devel] display09/src 
> - New directory
>                                       
>                                       
> 
>                                       Before you get too far - it's probably not a 
> good idea to name the "module" a
>                                       specific version number - as in "09".  I 
> suppose we can choose a better name
>                                       later since you're already in the process 
> though...
>                                       
>                                       Matt
>                                       
>                                       --- [EMAIL PROTECTED] wrote:
>                                       > Update of /cvsroot/displaytag/display09/src
>                                       > In directory 
> sc8-pr-cvs1:/tmp/cvs-serv18715/src
>                                       >
>                                       > Log Message:
>                                       > Directory /cvsroot/displaytag/display09/src 
> added to the repository
>                                       >
>                                       >
>                                       >
>                                       >
>                                       >
>                                       > 
> -------------------------------------------------------
>                                       > This SF.NET email is sponsored by: eBay
>                                       > Great deals on office technology -- on eBay 
> now! Click here:
>                                       > 
> http://adfarm.mediaplex.com/ad/ck/711-11697-6916-5
>                                       > 
> _______________________________________________
>                                       > displaytag-devel mailing list
>                                       > [EMAIL PROTECTED]
>                                       > 
> https://lists.sourceforge.net/lists/listinfo/displaytag-devel
>                                       >
>                                       >
>                                       
>                                       
>                                       __________________________________
>                                       Do you Yahoo!?
>                                       Yahoo! Calendar - Free online calendar with 
> sync to Outlook(TM).
>                                       http://calendar.yahoo.com
>                                       
> 
> 

-- 
John York
Software Engineer
CareerSite Corporation



-------------------------------------------------------
This SF.NET email is sponsored by: eBay
Great deals on office technology -- on eBay now! Click here:
http://adfarm.mediaplex.com/ad/ck/711-11697-6916-5
_______________________________________________
displaytag-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/displaytag-devel

Reply via email to