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

[Soheil H. Y. <soh...@gm...>]

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
>