Re: j2s-development] the state of the ASTScriptVisitor class

[Zhou R. <jo...@sj...>]

One thing should be mentioned before cleaning up SWT codes:
DO keep all those commented original codes (from Win32 SWT sources. You 
may need to compare sources with Win32 SWT sources to known which lines 
are Win32 SWT's). As we discussed before, those commented original 
sources will help us knowing which API is already implemented and which 
is not yet, and they also help us keeping update with SWT 3.2, 3.3.

All other commented codes can be removed. And new added or modified 
methods and fields (including modifying from "default" to "protected" or 
"public") should be commented.

In my opinions, there are no standards to audit cleaning-up. If the 
sources with some comments and documents can be well understood by other 
developers in one or several days, it would be OK. In order to achieve 
such a goal, maybe refactoring codes according to some well-known design 
patterns will help a lot.

Regards

Soheil Hassas Yeganeh wrote:
> Hi All,
>
> I think we should do clean up for all of our projects. As Zhou is
> starting to clean up j2s core , I will start cleaning up the SWT &
> Java Core projects.
> But I think there remains a problem. How should we audit the clean up
> itself ? Is it any pattern or practice that would help us cleaning the
> projects ? What is a good clean up ?
>
> Regards,
> Soheil
>
> On 12/2/06, Zhou Renjian <jo...@sj...> wrote:
>   
>> Hi Steven,
>>
>> Thanks a lot for pointing out the nasty things! Thanks your advices.
>>
>> It was my bad coding habit that made ASTScriptVisitor a mess. And this
>> habit also affected other classes. And I will spend coming days to clean
>> up those codes and add documents about ASTScriptVisitor and inner
>> JavaScript simulator mechanics.
>>
>> If there are other problems about the existed codes (Sure, there are
>> lots of nasty codes), please feel easy to post comments to point them
>> out. And I will try my best to clean things up.
>>
>> Thanks.
>>
>> Regards,
>> Zhou Renjian
>>
>> Steven Devijver wrote:
>>     
>>> All,
>>>
>>> I was planning to write my own ASTVisitor extension for j2s. The best
>>> way to get started for me seemed to look at the existing visitor
>>> classes. Thanks to the tutorial here
>>> http://j2s.sourceforge.net/articles/tutorial-extended-compiler.html it
>>> was easy to find the right class to look at: ASTScriptVisitor.
>>>
>>> Sadly I soon found out this class is in decay. It's a whooping 3430
>>> lines long (that's about 3000 lines in excess) and my guess is that
>>> about one in every 5 lines of code is commented out. As if this is not
>>> bad enough it gets worse.
>>>
>>> There are many very very deeply nested code blocks that make this
>>> class literally Chinese to anyone who wants to get to understand it.
>>> In fact, although I have quite some experience in performing code
>>> reviews I've never seen any class as poorly written as
>>> ASTScriptVisitor. And mind you, this is the first and only class I
>>> looked at in the entire j2s code base.
>>>
>>> Now, you may think I'm ranting. Consider this. I've selected one
>>> deeply nested code block out of many. This is its level of nesting.
>>> This code block that starts on line 2234 in ASTScriptVisitor is nested
>>> in:
>>>
>>> an else block in a while loop in an if block in an if block in an if
>>> block in an else block in an if block in an else block in a while loop
>>> in an if block in an if block in an if block in an else block in an
>>> else block in an if block in a method body.
>>>
>>> That's 16 (!!!) levels of nesting without encapsulation!!! And again,
>>> this is one example in a huge class in a big project.
>>>
>>> My recommendations are simple:
>>>
>>> - check in everything and create a tag in SVN today.
>>> - stop adding new features today so that the project is frozen. Don't
>>> add any new feature before the cleanup is complete.
>>> - in every class of the project, remove all code that is commented
>>> out. The only comments that are allowed are real comments in english,
>>> no code. Start with the classes that have the most lines and continues
>>> until you have checked every class in the codebase.
>>> - test all features of your project and make sure your code works as
>>> expected. Document exactly how you've tested every part of your code
>>> base. Create an issue for every bug you come across that's likely
>>> related to code that has been commented out.
>>> - at this point, stop making any modifications to your code base.
>>> - get together with all developers - either online to in real life -
>>> and start discussing the state of the code base. Start with the
>>> classes that have the most lines (the number of lines need to be
>>> recounted after all commented out code has been removed). Discuss why
>>> each class is as long as it is. Try to find out why these classes are
>>> so long. Try to come up with abstractions and encapsulations. Create
>>> some prototypes and verify if they are up to the job. If not improve
>>> them or come up with alternatives. If you found something interesting
>>> try it out on your code and see if it can improve the expressiveness
>>> of your code.
>>> - once you've selected a couple of code abstractions DOCUMENT THEM!!!
>>> - now attack your code base and implement the abstractions you''ve
>>> selected. Document each abstraction you implement like: ThisClass,
>>> line x to y, ThisAbstraction, This and That class created.
>>> - Re-test your codebase using the test descriptions you've written
>>> down before. Create an issue for every bug your discover that wasn't
>>> reported before.
>>> - fix the issues that were discovered after adding the abstractions.
>>> - now fix the issues that were discovered after the first test run.
>>>
>>> It will require some time but not more that one or two months. Anyway,
>>> if you don't do exactly as I've described your project is lost beyond
>>> repair and you'd better stop working on it today.
>>>
>>> Hope this helps. If you have any questions please feel free to contact me.
>>>
>>> Steven Devijver
>>>
>>> -------------------------------------------------------------------------
>>> 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
>>> _______________________________________________
>>> j2s-development mailing list
>>> j2s...@li...
>>> https://lists.sourceforge.net/lists/listinfo/j2s-development
>>>
>>>
>>>
>>>       
>> -------------------------------------------------------------------------
>> 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
>> _______________________________________________
>> j2s-development mailing list
>> j2s...@li...
>> https://lists.sourceforge.net/lists/listinfo/j2s-development
>>
>>     
>
> -------------------------------------------------------------------------
> 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
> _______________________________________________
> j2s-development mailing list
> j2s...@li...
> https://lists.sourceforge.net/lists/listinfo/j2s-development
>
>
>   

-- 

Regards,
Zhou Renjian

http://j2s.sourceforge.net/
Java2Script: Bridge of RCP to RIA
Reusing Java codes and tools into JavaScript