Hi All please find the comments and suggestions of the code review below

*Enhanced RXT functionalities
*
https://redmine.wso2.com/issues/494
https://redmine.wso2.com/issues/573

General
When using String Builder do not use + to add strings
When doing replaceAll methods try to add the strings then do the replaceAll


UIComponent
Change isJSGenerate variable, its not meaning full enough
Check the possibility to add methods to the base class if there
is functionality that is used in ever class for ex - name.reaplaceA();

CheckBox
Change isSkipName variable, its not meaning full enough
Replace the two returns with a single return by using conditional
if statement

DateFeild
Change move the this.id null check  to constructor

DropDown
Use for each, remove the value!=null not needed
Change the variable name of  value to selectedValue

OptionText
The name of the class is changed in the method check that.
See if the DropDown can be reused in this.

TextArea
change the variable mandatory into boolean

*Sample data populator*
https://redmine.wso2.com/issues/1033

CommandHandler
Several if statements can be simplified
Introduce an util methods to simplify the for loop in line 22 use
a hash-map to improve the code
remove the +e.getMessage() from exception messages
Write and utility method the handle client Options and options.setProperty()

Regards,
Pulasthi


On Thu, Jul 11, 2013 at 1:49 PM, Pulasthi Supun <[email protected]> wrote:

>  more details 
> »<https://www.google.com/calendar/event?action=VIEW&eid=aHJraDVkdXBudm1idXRza291NDNka21iMTQgZGV2QHdzbzIub3Jn&tok=MTcjcHVsYXN0aGlAd3NvMi5jb21jMmI1MzcwNTJjMzRiYzA2ODY5ZGY1Y2NmOTMzMTgyZmMwYmU5YmVh&ctz=Asia/Colombo&hl=en>
>  [Code Review] G-Reg new feature code reviews
> Crusible: 
> http://wso2.org/crucible/cru/WCP045-6<https://www.google.com/url?q=http%3A%2F%2Fwso2.org%2Fcrucible%2Fcru%2FWCP045-6&usd=2&usg=AFQjCNEigeeU4KlqYg2VgfNRbTCDhbOtmA>
> Crusible: 
> http://wso2.org/crucible/cru/PROD001-1<https://www.google.com/url?q=http%3A%2F%2Fwso2.org%2Fcrucible%2Fcru%2FPROD001-1&usd=2&usg=AFQjCNE733BNcsdkxeOJQMbYiKAmG22wDA>
> Crusible: 
> http://wso2.org/crucible/cru/WCP081-2<https://www.google.com/url?q=http%3A%2F%2Fwso2.org%2Fcrucible%2Fcru%2FWCP081-2&usd=2&usg=AFQjCNFLPSwPkAmxGC54cYA1jOYbuNxoaQ>
>
> https://redmine.wso2.com/issues/485<https://www.google.com/url?q=https%3A%2F%2Fredmine.wso2.com%2Fissues%2F485&usd=2&usg=AFQjCNGVim2u6Ia8olmTHe-G96RYlT0VXg>
> https://redmine.wso2.com/issues/573<https://www.google.com/url?q=https%3A%2F%2Fredmine.wso2.com%2Fissues%2F573&usd=2&usg=AFQjCNGVznX10bcc2AHOKTk0jCQhNNgBQQ>
> https://redmine.wso2.com/issues/494<https://www.google.com/url?q=https%3A%2F%2Fredmine.wso2.com%2Fissues%2F494&usd=2&usg=AFQjCNG85I6_X6082EqCnbltycNfAkWPhw>
> https://redmine.wso2.com/issues/1033<https://www.google.com/url?q=https%3A%2F%2Fredmine.wso2.com%2Fissues%2F1033&usd=2&usg=AFQjCNFZYf8Hike5QWFFjSxZyDaSE2bmOw>
> *When*
> ********Thu Jul 11, 2013 3pm – 5pm Colombo
> *Where*
> LK #58 5th Floor - Meeting room 
> (map<http://maps.google.lk/maps?q=LK+%2358+5th+Floor+-+Meeting+room&hl=en>
> )
> *Calendar*
> [email protected]
> *Who*
>  •
> [email protected] - organizer
> •
> Ajith Vitharana
> •
> Eranda Sooriyabandara
> •
> Danushka Fernando
> •
> Lasith Chandrasekara
> •
> Subash Chaturanga
> •
> Lalaji Sureshika
> •
> Senaka Fernando
> •
> Sanjeewa Malalgoda
> •
> [email protected]
> •
> Vijitha Kumara
>
> Going?   
> ***Yes<https://www.google.com/calendar/event?action=RESPOND&eid=aHJraDVkdXBudm1idXRza291NDNka21iMTQgZGV2QHdzbzIub3Jn&rst=1&tok=MTcjcHVsYXN0aGlAd3NvMi5jb21jMmI1MzcwNTJjMzRiYzA2ODY5ZGY1Y2NmOTMzMTgyZmMwYmU5YmVh&ctz=Asia/Colombo&hl=en>-
> Maybe<https://www.google.com/calendar/event?action=RESPOND&eid=aHJraDVkdXBudm1idXRza291NDNka21iMTQgZGV2QHdzbzIub3Jn&rst=3&tok=MTcjcHVsYXN0aGlAd3NvMi5jb21jMmI1MzcwNTJjMzRiYzA2ODY5ZGY1Y2NmOTMzMTgyZmMwYmU5YmVh&ctz=Asia/Colombo&hl=en>-
> No<https://www.google.com/calendar/event?action=RESPOND&eid=aHJraDVkdXBudm1idXRza291NDNka21iMTQgZGV2QHdzbzIub3Jn&rst=2&tok=MTcjcHVsYXN0aGlAd3NvMi5jb21jMmI1MzcwNTJjMzRiYzA2ODY5ZGY1Y2NmOTMzMTgyZmMwYmU5YmVh&ctz=Asia/Colombo&hl=en>
> *    **more options 
> »<https://www.google.com/calendar/event?action=VIEW&eid=aHJraDVkdXBudm1idXRza291NDNka21iMTQgZGV2QHdzbzIub3Jn&tok=MTcjcHVsYXN0aGlAd3NvMi5jb21jMmI1MzcwNTJjMzRiYzA2ODY5ZGY1Y2NmOTMzMTgyZmMwYmU5YmVh&ctz=Asia/Colombo&hl=en>
>
> Invitation from Google Calendar <https://www.google.com/calendar/>
>
> You are receiving this courtesy email at the account [email protected] because
> you are an attendee of this event.
>
> To stop receiving future notifications for this event, decline this event.
> Alternatively you can sign up for a Google account at
> https://www.google.com/calendar/ and control your notification settings
> for your entire calendar.
>
> _______________________________________________
> Dev mailing list
> [email protected]
> http://wso2.org/cgi-bin/mailman/listinfo/dev
>
>


-- 
--
Pulasthi Supun
Software Engineer; WSO2 Inc.; http://wso2.com,
Email: [email protected]
Mobile: +94 (71) 9258281
Blog : http://pulasthisupun.blogspot.com/
Git hub profile: https://github.com/pulasthi
_______________________________________________
Dev mailing list
[email protected]
http://wso2.org/cgi-bin/mailman/listinfo/dev

Reply via email to