Re: j2s-development] the state of the ASTScriptVisitor class
Brought to you by:
zhourenjian
Menu
▾
▴
Re: j2s-development] the state of the ASTScriptVisitor class
|
From: Soheil H. Y. <soh...@gm...> - 2006-12-03 06:30:46
|
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 > |
