code reviews

Download Code reviews

Post on 23-Sep-2014




3 download

Embed Size (px)




PowerPoint Presentation

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."

Loose couple:FacadeSpringMessaging

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 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 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

Type safeCasting

Thread safe

OOM (static), file description handler, release resources (File, DBConnection)

SQL injection

Single point of failure

Mitigation of serialization:- Dont extend Implement readObject and writeObject as final methods that throw IOException- If serialize you must: use transient or use plus Encryption

21 secure code

Have Fun and win