Post on 23-Sep-2014
Embed Size (px)
Every Programmer Should Know Code ReviewsRoger XiaJuly 2012$ whoami Programmer
Programming & Code review Programming isTaking an algorithmChoosing a languageUsing that language to implement algorithm and solve problems
Code review is?Why? Increase Quality & Reduce Defects Improve readability Share knowledge in team Know your workmate better! Two Wrongs Can Make a Right
NOT personal attack! NOT architect reviews everything
methodology Team review (Planned 1-2 hour/week, Clear roles) Pair programming (Share knowledge, 1 task) Walkthrough (Author leads, reviewers take notes, higher level) Peer review (Asynchronous)
Gerrit Reaction & Ask questions
PreparationCode ConventionsFindbugsTestedTest case
Take care of naming conventionspelling mistakes business logic refactoring performance security (attack, thread safe)
RefactoringRefactoring modifies software to improve its readability, maintainability, and extensibility without changing what it actually does.
Martin Fowler uses code smells to identify when to refactor.
Boss: "Refactoring is an overhead activity - I'm paid to write new, revenue generating features."
9Code smellsBad namesDuplicate codeLong methodLarge classLong parameter listTemporary fieldSpeculative GeneralityData ClassDont flood log
Temporary fieldAn attribute of an object is only set in certain circumstances; but an object should need all of its attributes
Speculative GeneralityOh I think we need the ability to do this kind of thing someday
Data ClassThese are classes that have elds, getting and setting methods for the elds, and nothing else; they are data holders, but objects should be about data AND behavior
10Use Meaningful Names
http://www.slideshare.net/srikanthps/practices-for-becoming-a-better-programmer-presentation11Meaningful NamesClass namesShould be nouns or noun phrases.Examples: Car, Account, DataRetrievalService, AddressParser
Method namesShould be verb or verbPhrasesExamples: parseData, deletePage, saveMethods that return boolean values should sound like question. Examples: isAuthenticated, hasNoErrors, isEmpty
Interface and ImplementationICache LRUCacheIExport ExportService
The Art of Readable codeThe book!
I want to point out:Use blank to separate logic block.
Comments for complex process, algorithm, reasons
Are we writing comments because our code is unclear?Will you keep the comments up-to-date whenever code is updated?
14Aiming for simplicityDo one thing in a function (simple responsibility)Have no side effects.
Prefer exceptions to return codes.
Format your code.
15DRY -- Dont repeat yourselfDuplicated code should be avoided.Object Orientation, Abstract!Design pattern!
If there is a bug in the code or code requires changes, then, one has to change it at multiple places.
16OO PrinciplesSimple responsibility principle: Class should have one and only one reason to change.
Encapsulation: Modules should not know internal details of objects it manipulates.
Polymorphism -- Liskovs substitution principle: A subclass can be used as an argument where a base class is expected.
Open-closed principle: Class should be open for extention, but closed for modification.Design Patterns
http://www.lifeyun.com/design-pattern-diagram.html18Pay Attention to PerformanceJAVA: JVM usageDont create object in loop
Use ArrayList, HashMap etc as opposed to Vector, Hashtable etc (synchronized) where possible. Even better is to use just arrays where possible.
Set initial capacity of a collection (e.g. ArrayList, HashMap) and StringBuffer/StringBuilder appropriately.
Concurrent Collection, Lock
Lazy load or multi-threading where applicable.
Cache (LRUCache, Distributed Cache)
19Pay Attention to Performance
20Pay Attention to SecuritySandbox (security manager, access manager, Classloaders, policies)
Scope: Access modifier to help protect your classes, methods, fields.public, protected, private, packageExceptions: object serialization, reflection,
Immutable classfinalStringInsecure direct object reference of mutable object
OOM (static), file description handler, release resources (File, DBConnection)
Single point of failure
Mitigation of serialization:- Dont extend java.io.serializable- Implement readObject and writeObject as final methods that throw IOException- If serialize you must: use transient or use java.io.Externalizeable plus Encryption
21 secure code
Have Fun and winhttp://rosettacode.org/wiki/Rosetta_Code