Code Review Component - Registry Search
Reviewer - Lahiru

AdvancedSearchResults.java
-------------------------------------------
Use for each loop instead of for loop
Remove the data binding part within the loop. Instead of the conditional
checks use a bean class.
Use two arrays and compare property names and values.
The above improvements will add a performance improvement.

Use the generics properly.
LOC 182 Invert the conditional check
Replace for loops with for-each loops (it will add a performance gain as
well)

AdvancedResourceQuery.java
---------------------------------------------
Have a executor() method.
Pass in the query bean to the executor. This way it is more elegant.
Separate out query meta data and query
Use arrays.
Currently, the search is specifically written to do the search. The search
should accept the search criteria.
User should not know about the underlying mechanism of the search.
Registry does not have proper apis for searching. Therefore you have worked
around that.
For random search storing the query in database in not acceptable.
Have to check whether the query path exist in the registry. If not calculate
the SQL query.
When having a if {} else {} condition, try to move the logic to the if{}
condition instead of having the logic in els{} block. Re-factor the code.
Remove the unnecessary boolean checks (eg. noCustomSearch). Write the logic
in a such a way that it is clearly understood by a another person.
Add code level comments explaining the logic. It will be easier for a person
to understand.
Separate out concerns into methods. When naming methods, try to give
descriptive names to the methods.


Improvements to the existing implementation
------------------------------------------------------------------
Extend the UI to support addition of multiple properties
Should support adding new media types to the registry (Give a nice interface
to the user)

Future
---------
Need to review the Searching mechanism
Try to draw a logical diagram in use-cases for the search mechanism
Let's do another round of review once we integrate the proposed changes to
the component.


On Thu, Feb 3, 2011 at 12:57 PM, Lahiru Gunathilake <lah...@wso2.com> wrote:

> cruble project can be find here[1].
>
> [1]https://wso2.org/crucible/cru/COMPONENT-35
>
> Lahiru
>
> On Thu, Feb 3, 2011 at 11:33 AM, Lahiru Gunathilake <lah...@wso2.com>wrote:
>
>>  This event has been changed.
>> more details 
>> »<https://www.google.com/calendar/event?action=VIEW&eid=bXI4NTVka2xjN2tzbHE0YW1xNGxqdGEzcm8gY2FyYm9uLWRldkB3c28yLm9yZw&tok=MTUjbGFoaXJ1QHdzbzIuY29tYjM0ZDBhODFmZWVjZDQ2OWViMmEwYzUwZGU5NzI2NmIxZWM4ZGE5MA&ctz=Asia%2FColombo&hl=en>
>> Group B : Code review (Registry Search Service)
>> Code review will conduct for Registry Search Service. I will create a
>> curcible project by the time we start the code review.
>> *When*
>> *Changed: *Thu Feb 3 1pm – 2pm Colombo
>> *Where*
>> 4th floor (map <http://maps.google.com/maps?q=4th+floor&hl=en>)
>> *Calendar*
>> carbon-dev@wso2.org
>> *Who*
>> •
>> Lahiru Gunathilake - organizer
>> •
>> Milinda Pathirage
>> •
>> Ajith Vitharana
>> •
>> Chamara Silva
>> •
>> Nuwan Bandara
>> •
>> carbon-dev@wso2.org
>> •
>> Heshan Suriyaarachchi
>> •
>> Manjula Rathnayake
>> •
>> Sarasi Munasinghe
>> •
>> Chamara Ariyarathne
>> •
>> Ruwan Linton
>> •
>> Sumedha Rubasinghe
>> •
>> Nandika Jayawardana
>>
>> Going?   
>> *Yes<https://www.google.com/calendar/event?action=RESPOND&eid=bXI4NTVka2xjN2tzbHE0YW1xNGxqdGEzcm8gY2FyYm9uLWRldkB3c28yLm9yZw&rst=1&tok=MTUjbGFoaXJ1QHdzbzIuY29tYjM0ZDBhODFmZWVjZDQ2OWViMmEwYzUwZGU5NzI2NmIxZWM4ZGE5MA&ctz=Asia%2FColombo&hl=en>-
>> Maybe<https://www.google.com/calendar/event?action=RESPOND&eid=bXI4NTVka2xjN2tzbHE0YW1xNGxqdGEzcm8gY2FyYm9uLWRldkB3c28yLm9yZw&rst=3&tok=MTUjbGFoaXJ1QHdzbzIuY29tYjM0ZDBhODFmZWVjZDQ2OWViMmEwYzUwZGU5NzI2NmIxZWM4ZGE5MA&ctz=Asia%2FColombo&hl=en>-
>> No<https://www.google.com/calendar/event?action=RESPOND&eid=bXI4NTVka2xjN2tzbHE0YW1xNGxqdGEzcm8gY2FyYm9uLWRldkB3c28yLm9yZw&rst=2&tok=MTUjbGFoaXJ1QHdzbzIuY29tYjM0ZDBhODFmZWVjZDQ2OWViMmEwYzUwZGU5NzI2NmIxZWM4ZGE5MA&ctz=Asia%2FColombo&hl=en>
>> *    more options 
>> »<https://www.google.com/calendar/event?action=VIEW&eid=bXI4NTVka2xjN2tzbHE0YW1xNGxqdGEzcm8gY2FyYm9uLWRldkB3c28yLm9yZw&tok=MTUjbGFoaXJ1QHdzbzIuY29tYjM0ZDBhODFmZWVjZDQ2OWViMmEwYzUwZGU5NzI2NmIxZWM4ZGE5MA&ctz=Asia%2FColombo&hl=en>
>>
>> Invitation from Google Calendar <https://www.google.com/calendar/>
>>
>> You are receiving this courtesy email at the account 
>> carbon-dev@wso2.orgbecause 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.
>>
>> _______________________________________________
>> Carbon-dev mailing list
>> Carbon-dev@wso2.org
>> http://mail.wso2.org/cgi-bin/mailman/listinfo/carbon-dev
>>
>>
>
>
> --
> Lahiru Gunathilake
> Senior Software Engineer - WSO2 Inc. www.wso2.com
>
> Email:lah...@wso2.com <email%3alah...@wso2.com> Blog: www.lahiru.org
> Mobile: +94716381143
>
> Lean . Enterprise . Middleware
>
>


-- 
Regards,
Heshan Suriyaarachchi
Software Engineer
WSO2 Inc.; http://wso2.com/

Blog: http://heshans.blogspot.com/
_______________________________________________
Carbon-dev mailing list
Carbon-dev@wso2.org
http://mail.wso2.org/cgi-bin/mailman/listinfo/carbon-dev

Reply via email to