Send mail to the author(s) E-mail

# Wednesday, 17 December 2014
( Code Reviews | NDC )

Inspirational guy!

  • Design principles
  • Where
  • Why?
    • So code is maintainable
  • When
  • DRY
    • Every bit of knowledge should have single authoritative thing
  • SRP
    • Long methods
    • SLAP – Single level of abstraction
  • TDA
  • OCP
  • LSP
  • DIP

Feedback code reviews

Pairing is good

Make it work, make it better real soon

| | # 
# Tuesday, 03 June 2014


  • Improve the quality of the team
  • Improve communication
  • Improve quality of code
  • Find coding problems


  • Find things to laud and to fix
  • Look for common problems, not one-off issues (so learning)
  • Review 200-400 lines at a time (in an hour!)
  • No code is perfect
    • maintainable code is a feature

Use MS Word commenting feature?

How not to do it

  • Avoid emotions
  • Don't focus on blame
  • Don't redesign code
  • Don't judge based on how your would have coded it (ie is this code maintainable and proficient)
  • Don't include one off problems (trying to find patterns to fix)

Common Problems in C#

Magic Strings

eg if writing to a file, and use filename twice, then this is bad.


Not Invented Here Syndrome

  • Buy or borrow - don't build unless you're doing something very special
    • eg caching
    • ORM
    • MVC Filters with Business Logic (AOP style)

Too Many Layers

  • eg DAL

Bad to have these diagrams.. want to simplify

For huge system, maybe this is good.  For 30 users system..overkill!  Slowing us down.. productivity to clients.

Lack of Unit Tests

You can't determine code quality without tests

  • Manual testing is good, but doesn't scale
  • If you're not running tests, your users are your testers
  • Write tests for client and server side code
  • Most code small originated in lack of testing
  • Unit tests are more valuable for regression than creation

Continuous Integration

Building code frequently improves code quality

  • Integration problems are surfaced earlier
  • Running automated unit tests helps find regression bugs
  • Breaking the build should be enforced by shame, not reprimands!
  • Large teams this becomes more important
  • Should know the state of the code
    • auto deployment to test servers

Lack of Dependency Injection


Circular Dependencies

eg logger in repository etc

Lazy Loading on by default

Lazy Load to false

| | #