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

[Steven D. <ste...@gm...>]

I think that's ok as long as somone who's new to the j2s code can
understand that he or she can safely ignore this code.

Steven

On 12/4/06, Soheil Hassas Yeganeh <soh...@gm...> wrote:
> Hi All,
>
> I think we need those SWT codes, but we should discuss how to keep
> them in the HEAD. Zhou said it should remain in the way they do now.
> Steven said we should keep it in another branch.
>
> But I suggest to keep them with a convention. I mean that if we keep
> those comments with a meta data (annotation). For example for those
> win32 commented codes ( that I think they will really help us
> developing SWT ) we should write :
> /*WIN32 NATIVE*
> * .....
> */
>
> /*GTK NATIVE*
> ...
> */
>
> I think marking the codes this way will bring us more productivity
> than just diffing the SVN branches.
>
> Regards,
> Soheil
> On 12/3/06, Zhou Renjian <jo...@sj...> wrote:
> > Hi Steven,
> >
> > Thanks for you detailed suggestions.
> >
> > Here is a quick reply:
> >
> > I think Java2Script SWT's commented codes should stay in sources for
> > some more time. I still need them: I need to look at the sources knowing
> > the differences of Java2Script SWT and native SWT without comparisons.
> > By the way, SVN connection is still considered slow. And using SVN may
> > also require diff3.
> >
> > ASTScriptVisitor is being refactored these days. If you are interested
> > in ASTScriptVisitor and would like to help all whole compiler things,
> > you can keep update with SVN and give feedbacks about those "Chinese"
> > codes. BTW: I am a Chinese, and I can read and write Chinese. So
> > feedbacks from English or other languages will be important to me. :)
> > Maybe ASTScriptVisitor is not being refactored in your way of
> > understanding Java2Script compiler. But at least it will be being
> > refactored to a level that is more understandable and extensible.
> >
> > - code that inspects the Java AST objects
> > - code that generates JavaScript.
> >
> > Good suggestions. I think I am in the direction.
> >
> > Regards
> >
> > Steven Devijver wrote:
> > > Hey guys,
> > >
> > > It's good to hear you want to clean up the project. I've used j2s and
> > > I think it's a very cool tool with a big potential for the future. You
> > > guys have done a lot of great work and it would be a shame to see it
> > > go to waste.
> > >
> > > About the win32 code that's commented out: I understand you want to
> > > keep these comments around for reference. However, they cannot stay in
> > > the HEAD if this code is not going to be actually used.
> > >
> > > The reason for this is that this commented out code is going to
> > > confuse developers that look at the j2s source code like it confused
> > > me. There are several options to keep this code around without have to
> > > keep it where it is now.
> > >
> > > The first option is - like I suggested - to create a tag in SVN before
> > > you start the cleanup. This way you can easily do a compare in Eclipse
> > > between the HEAD and the tag to have a look at the old comments.
> > >
> > > Another, more flexible approach is to create a branch before the
> > > cleanup that you can reference to whenever you want to inspect the
> > > code - including the comments - before the cleanup.
> > >
> > > Whatever you do, you need to somehow mark the current code before the
> > > cleanup to compare in case of bugs or mysterious behavior.
> > >
> > > Regarding what abstractions to use: the way to find them that works
> > > best for me is to print out a class like ASTScriptVisitor and mark
> > > with color markers the code in this class that does a specific thing.
> > > In the case of ASTScriptVisitor you will find at least two types of
> > > code:
> > >
> > > - code that inspects the Java AST objects
> > > - code that generates JavaScript.
> > >
> > > You can mark these distinct patches of code each in its own code, for
> > > example yellow for code that works with AST objects and orange for
> > > code that generates JavaScript.
> > >
> > > Once all your code is colored you'll have a much better overview over
> > > the different kinds of behavior. Now, the goal is to have not more
> > > than one kind of behavior in each class. So inspect AST objects AND
> > > generating JavaScript is one kind of behavior too many in
> > > ASTScriptVisitor.
> > >
> > > The goal is to select one behavior that can stay in ASTScriptVisitor -
> > > I think inspecting the AST objects is the most sensible choice - and
> > > moving the other kinds of behavior each into their own classes.
> > >
> > > For example, generating Javascript code is a great candidate to move
> > > to a separate class because it's such a specific behavior. It will
> > > give you a much better view on each kind of behavior and that's the
> > > virtue of object-oriented development. You should take advantage of
> > > that.
> > >
> > > Regarding design patterns, you shouldn't worry about that too much.
> > > Once you start thinking about abstractions and refactoring you'll
> > > automatically end up with Factories and Visitors and Delegates and
> > > Adapters. They are pretty natural for object-oriented development.
> > >
> > > Hope this helps. Now, the task you're about to embark on is not an
> > > easy one and it's perfectly normal to not get it right the first time
> > > around. So my advice is to check in regularly, even if you know that
> > > what you've done is not correct. You can always roll back to a
> > > previous version. Just keep your ideas so that you can inspect them at
> > > later times.
> > >
> > > A specific approach that didn't work in one place may be the ideal
> > > solution in another. So let SVN become your collective memory, from my
> > > experience it will make the job a lot easier.
> > >
> > > Also, you will need to talk a lot before you start making changes.
> > > You're much smarter with 2 or 3 than by yourself. And even if you
> > > can't get an agreement on how to continue, you can always decide to
> > > give a certain approach that you think might work a try and evaluate
> > > it afterwards. There will be a lot of unknowns and the only way to
> > > advance is to try and verify the result.
> > >
> > > Keep the courage, you're guys are doing great.
> > >
> > > Steven
> > >
> > > On 12/3/06, Zhou Renjian <jo...@sj...> wrote:
> > >
> > >> 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
> > >>
> > >> -------------------------------------------------------------------------
> > >> 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
> >
> > -------------------------------------------------------------------------
> > 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
>
>