Sunday, May 14, 2006
Sanaa code review (05-11-2006 ) + Asp.Net 2.0 Site Maps and Navigation.
Sese seun@splasherstech.com handled code review for Sanaa and Tunde tunde@splasherstech.com handled geekify for same.
Code review was a quick post-motem (I know Stan hates that word) of Relius Sync. Sese did a good job introducing the project and explained his architecture and why he made certain decisions. He went on and stepped through the code to display actual implementation.
Then we butchered the codes, not really, we discussed some of his implementation rights and wrongs from different perspectives and known rights and wrongs, otherwise known as best practices.
Part of the observations, which we hope will be refactored too include but aren’t limited to:
The absence of XML documentations, especially for public methods. Seun felt all his methods are descriptive (SeunWillNowUpdateRowsBackToDatabase() ). We all agreed only his send mail method required no Xml comments. Devs also mentioned appropriate naming could make tools like GhostDoc generate descriptive Xml comments + VS 2005 gives warnings when public methods don’t carry XML comments.
The configuration settings are wrapped into a Settings created by developer. VS 2005 allows you to add a settings file with visual designer support. Could be re-inventing the wheel.
Some guys think the method names are unprofessional. DoSynchronise à Synchronize since synchronize is a verb. IsErrorOccured à ErrorOccured. Sanaa argues Is___ tells them immediately it’s a Boolean result expected, well, whatever.
average adherence to naming conventions, Pascal, underscore hybrid mix.
The versioning used for display was hard-ceded. This should be resident in AssemblyInfo.cs. A call like Application.Version would return the version and the version gets built into the assembly too.
Handling code logics in exception and nesting exception blocks are practices that should be discouraged. Nesting exceptions has negative impact on performance.
Some methods are really lengthy. We suggested 20 tops 25 lines per methods. Making codes more atomic makes it easy to read, gives smoother flow and becomes more maintainable etc.
The main geekify introduced the SiteMap features of ADO.Net 2.0 and menu controls. A short demo showed binding Xml sources to the tree view control and using site navigation controls. We hope to see posts on this on the team blog some blogs somewhere.
Great job Sanaa guys, Buchao! ®.
I hope I have spoken for all.
Code review was a quick post-motem (I know Stan hates that word) of Relius Sync. Sese did a good job introducing the project and explained his architecture and why he made certain decisions. He went on and stepped through the code to display actual implementation.
Then we butchered the codes, not really, we discussed some of his implementation rights and wrongs from different perspectives and known rights and wrongs, otherwise known as best practices.
Part of the observations, which we hope will be refactored too include but aren’t limited to:
The absence of XML documentations, especially for public methods. Seun felt all his methods are descriptive (SeunWillNowUpdateRowsBackToDatabase() ). We all agreed only his send mail method required no Xml comments. Devs also mentioned appropriate naming could make tools like GhostDoc generate descriptive Xml comments + VS 2005 gives warnings when public methods don’t carry XML comments.
The configuration settings are wrapped into a Settings created by developer. VS 2005 allows you to add a settings file with visual designer support. Could be re-inventing the wheel.
Some guys think the method names are unprofessional. DoSynchronise à Synchronize since synchronize is a verb. IsErrorOccured à ErrorOccured. Sanaa argues Is___ tells them immediately it’s a Boolean result expected, well, whatever.
average adherence to naming conventions, Pascal, underscore hybrid mix.
The versioning used for display was hard-ceded. This should be resident in AssemblyInfo.cs. A call like Application.Version would return the version and the version gets built into the assembly too.
Handling code logics in exception and nesting exception blocks are practices that should be discouraged. Nesting exceptions has negative impact on performance.
Some methods are really lengthy. We suggested 20 tops 25 lines per methods. Making codes more atomic makes it easy to read, gives smoother flow and becomes more maintainable etc.
The main geekify introduced the SiteMap features of ADO.Net 2.0 and menu controls. A short demo showed binding Xml sources to the tree view control and using site navigation controls. We hope to see posts on this on the team blog some blogs somewhere.
Great job Sanaa guys, Buchao! ®.
I hope I have spoken for all.
# posted by Unknown @ 5:42 AM 
