Hi All, Following are some of the points and suggestions brought up at the code review, please feel free to add up the missing points.
- Rename the modules. The name "Deployment" doesn't give the correct meaning, suggestion was to rename it to SCIM Impl. - Remove the invalid annotations in the source. - Instead of using a single validator for SCIM object validation, use multiple validators for each type. Use an abstract validator and then create concrete validators for each type. - Extract a private method from the attribute encoder methods (simple, complex etc) and move all the common stuff to over there. - Write test cases for all the Parsers. - In the createUser() method, do all the sanity checkups at the beginning of the method. - At the getEncoder() method of the AbstractResourceEndpoint, the method should throw an exception if the encodeMap object is empty. - Do log the code. Use commons logging. - Warnings must be used at the removeAnyReadOnlyAttributes() method. - Try to optimize the nested if statements in the removeAnyReadOnlyAttributes() method. - Move encodeSCIMException() method to another Utill class instead of having it in an Abstract class. - Improve the Java Docs. Mention when exceptions are thrown etc. - Try to reuse HTTP constants instead of redefining them if possible. - The length check is unnecessary in the setEmails() method. - Write test cases for the basics of the SCIM protocol. Thanks & regards, -Suresh On Tue, Apr 17, 2012 at 7:21 PM, Hasini Gunasinghe <has...@wso2.com> wrote: > more details > »<https://www.google.com/calendar/event?action=VIEW&eid=bDdsZHBlbGUwNmk0cDhsOXQ1ZDJnZDNrMTAgc3VyZXNoQHdzbzIuY29t&tok=MTUjaGFzaW5pQHdzbzIuY29tZTNhZDYyZGE3ZjAyOGEwYTEyNThlNWZkZjI1Nzg4NWU0MzViODEwMQ&ctz=Asia/Colombo&hl=en> > [Integration TG] Code Review > Details: > Project being reviewed: WSO2 Charon > Link to crucible > project:http://wso2.org/crucible/cru/WSCC001-1<http://www.google.com/url?q=http%3A%2F%2Fwso2.org%2Fcrucible%2Fcru%2FWSCC001-1&ust=1334677897020000&usg=AFQjCNE5LnfBOQT6lBrlICwSfaHDdPVX4A> > Other info - Link to the related class diagrams: > https://svn.wso2.org/repos/wso2/trunk/commons/charon/documentation/<http://www.google.com/url?q=https%3A%2F%2Fsvn.wso2.org%2Frepos%2Fwso2%2Ftrunk%2Fcommons%2Fcharon%2Fdocumentation%2F&ust=1334677897020000&usg=AFQjCNG5MSVi2fM-Ho-hC3VJUN1UXUsfoQ> > *When* > Wed Apr 18 10am – 11am Colombo > *Where* > LK #58 5th Floor - Meeting room > (map<http://maps.google.lk/maps?q=LK+%2358+5th+Floor+-+Meeting+room&hl=en> > ) > *Calendar* > sur...@wso2.com > *Who* > • > has...@wso2.com - organizer > • > Asela Pathberiya > • > Johann Nallathamby > • > Thilina Buddhika > • > Hiranya Jayathilaka > • > dev@wso2.org > • > Suresh Attanayaka > > Going? > ***Yes<https://www.google.com/calendar/event?action=RESPOND&eid=bDdsZHBlbGUwNmk0cDhsOXQ1ZDJnZDNrMTAgc3VyZXNoQHdzbzIuY29t&rst=1&tok=MTUjaGFzaW5pQHdzbzIuY29tZTNhZDYyZGE3ZjAyOGEwYTEyNThlNWZkZjI1Nzg4NWU0MzViODEwMQ&ctz=Asia/Colombo&hl=en>- > Maybe<https://www.google.com/calendar/event?action=RESPOND&eid=bDdsZHBlbGUwNmk0cDhsOXQ1ZDJnZDNrMTAgc3VyZXNoQHdzbzIuY29t&rst=3&tok=MTUjaGFzaW5pQHdzbzIuY29tZTNhZDYyZGE3ZjAyOGEwYTEyNThlNWZkZjI1Nzg4NWU0MzViODEwMQ&ctz=Asia/Colombo&hl=en>- > No<https://www.google.com/calendar/event?action=RESPOND&eid=bDdsZHBlbGUwNmk0cDhsOXQ1ZDJnZDNrMTAgc3VyZXNoQHdzbzIuY29t&rst=2&tok=MTUjaGFzaW5pQHdzbzIuY29tZTNhZDYyZGE3ZjAyOGEwYTEyNThlNWZkZjI1Nzg4NWU0MzViODEwMQ&ctz=Asia/Colombo&hl=en> > * **more options > »<https://www.google.com/calendar/event?action=VIEW&eid=bDdsZHBlbGUwNmk0cDhsOXQ1ZDJnZDNrMTAgc3VyZXNoQHdzbzIuY29t&tok=MTUjaGFzaW5pQHdzbzIuY29tZTNhZDYyZGE3ZjAyOGEwYTEyNThlNWZkZjI1Nzg4NWU0MzViODEwMQ&ctz=Asia/Colombo&hl=en> > > Invitation from Google Calendar <https://www.google.com/calendar/> > > You are receiving this email at the account sur...@wso2.com because you > are subscribed for invitations on calendar sur...@wso2.com. > > To stop receiving these notifications, please log in to > https://www.google.com/calendar/ and change your notification settings > for this calendar. > -- Suresh Attanayake Software Engineer; WSO2 Inc. http://wso2.com/ Blog : http://sureshatt.blogspot.com/ Twitter : https://twitter.com/sureshatt LinkedIn : http://lk.linkedin.com/in/sureshatt Mobile : +94755012060,+94770419136,+94710467976
_______________________________________________ Dev mailing list Dev@wso2.org http://wso2.org/cgi-bin/mailman/listinfo/dev
