How To Improve Code Quality


Follow along at:

Created by Kip Streithorst / @itsnull

WARNING - Opinions!!

  • These are MY opinions
  • Stop me, ask questions
  • Sorry, no code to see here

How To Improve Code Quality

  • What is quality code?
  • Ways to improve it

What is quality code?

  • Satisfies customer requirements
  • Easy to change
    • Defining feature of software compared to hardware

How to satisfy customer requirements?

  • The great question, is there an answer?
  • Yeah, know what you don't know. Be agile.
  • Deliver early, deliver often.
  • Get feedback early, get feedback often.
  • Reprioritize as needed.

How to deliver early, deliver often

  • Automate it. No, really automate it.
  • Continuous integration - automated builds on check-in.
  • Continous deploy - automated or push button deployments all the way to production
  • Deploy multiple times a month, multiple times a day.

What is quality code?

  • Satisfies customer requirements
  • Easy to change
    • Defining feature of software compared to hardware

How to make code easy to change

  • Be able to assert code behaves the same before and after a change
  • Code is maintainable
  • Code is readable

How to assert behavior hasn't changed?

  • Write tests
  • Unit tests, integration tests, system tests, etc.
  • Make them easy to run
  • Automate them, e.g. continuous integration, continuous deploy

How to ensure maintainability?

  • Afraid to change it? Then it's not maintainable
  • Can't explain it? Then it's not maintainable
  • Can't easily debug it? Then it's not maintainable

Unit tests can help with maintainability

  • Changing it? Tests help prevent regressions
  • Explain it? Tests provide example of happy paths and sad paths
  • Debug it? Debug the test to get directly to a piece of lower level code

However, tests aren't the whole answer

  • Tests only assist with their focus area
  • To improve maintainability, we need to improve readability

What does readable code mean?

  • Can code be universally readable?
  • Are all books universally readable?

What does readable code mean? (pt. 2)

  • Chemistry Book
  • Knitting Book
  • Economics Book
  • Software Design Pattern Book
  • Are all of those equally readable for everyone?
  • No, they have required knowledge of the domain that contributes to the readability

What does readable code mean? (pt. 3)

  • We write custom software to deliver value for a domain
  • So each piece of software has required domain knowledge
  • So, readability of code isn't universal

What does readable code mean? (pt. 4)

  • You need someone with domain knowledge first
  • Then you can evaluate readability
  • So, to improve readability, you need another team member to read it.
  • So... let's talk about code reviews in depth

What are Code Reviews?

  • Another member of the team reads the code and provides feedback
  • That feedback is incorporated back into the code
  • Repeat until team is satisifed
  • Think back to editing a writing assignment

Why do Code Reviews?

  • Increase readability
  • Keep scary code out
  • Keep code that can't be explained out
  • Keep code that can't be debugged out
  • ... Improve Code Quality

Why do Code Reviews? (pt. 2)

  • Other Benefits as well
  • Find bugs earlier
  • Intrinsic cross training
  • Break down of silos
  • Mentor/on board team members

What isn't a code review

  • Not a performance or review metric for software engineers
  • Not a way to belittle others
  • Not a way to settle scores
  • Not a way to rubber stamp each others work
  • Not just the job of one person on the team

Mindset of the coder

  • Review comments are an opening line in a discussion
  • Review comments can be ignored
  • Review comments are NOT an order or directive to be blindly followed
  • You still own the completion of the task/feature, not the reviewer

Mindset of the coder

  • You are NOT your code
  • If a suggestion results in undoing of your previous work, that's ok
  • No ONE is perfect, you will miss something
  • The quantity of feedback has NOTHING to do with the size of your change
  • A suggestion might be for code you didn't change, that's ok
  • Remember, the goal is to improve the code quality

Mindset of the reviewer

  • You are providing feedback, not orders or directives
  • Not all of your feedback may be accepted or acted upon
  • Never a need for personal attacks or inappropriate language, it distracts from your point and wastes time
  • Try not to pre-filter your feedback, if you think of it, put it down
  • Try and review equally, regardless if you are reviewing team lead's code or new team member's code

What is the reviewer looking for?

  • Does code implement the actual task/feature?
  • Does code actually run? Do you know how to run or debug code?
  • Have you tried to break code? Does code handle errors?

What is the reviewer looking for? (pt. 2)

  • Is code readable?
  • Is code understandable?
  • Would you be afraid to maintain the code?

What is the reviewer looking for? (pt. 3)

  • Is there anything missing?
  • Should similiar changes be made elsewhere?
  • Should some changes NOT be made?

What is the reviewer looking for? (pt. 4)

  • Does code have performance issues?
  • Does code have security issues?
  • Does code follow applicable standards?

What is the reviewer looking for? (pt. 5)

  • Does code follow team patterns?
  • Does code follow team naming conventions?
  • Does code follow team formatting conventions?

What is the reviewer looking for? (pt. 6)

  • Does code have unit tests?
  • Do unit tests follow team conventions?
  • Are unit tests testing happy paths?
  • Are unit tests testing sad paths?

What gets reviewed?

  • Every commit to the project
  • If you add exceptions to the above, the loop hole will get abused
  • If it really is simple, it really will be quick to review

Who should do reviews?

  • At least one other coder on the project
  • Coders should be reviewers and vice-versa
  • Silo'ing into coder only, reviewer only will demotivate and create adversarial relationships
  • More reviewers is good if the code is critical

How are reviewers selected?

  • Whatever works for your team
  • I've seen self assignment work
  • I've seen team lead assignment work
  • Just check to ensure reviews are actual resulting in code quality improvements

When do reviews happen?

  • Before the code is committed to master or trunk
  • After that and any leverage is lost
  • We want tangible code quality improvements not a list of future tasks

When do reviews happen (pt. 2)?

  • The reviewer should review independently
  • Don't review in a real-time meeting
  • Reviewer needs to be able to make multiple passes
  • Reviewer needs to be able to take breaks
  • Reviewer needs to be able to research
  • Don't timebox a review before it starts. If it's going long, ask why

Resolving disputes

  • Try and let coder and reviewer work it out
  • Bring members of the team into the discussion, drive to consensus
  • Last resort is decision from on high

Scope of Review Comments

  • Will the fix radically increase the size or risk of the overall change?
  • Maybe split it into another commit and do it separately, either before or after this change
  • Don't use this an excuse to NOT do something that should be done

Code review is too long?

  • Is it? Are you still getting value?
  • Should the change have been smaller to start with? Do that next time

Code Review Mindset

  • Leave it better than you found it
  • Don't take feedback personally
  • Almost every review should have at least ONE comment
  • No ONE is perfect

Starting with code reviews

  • Just start and adjust as you go
  • The first reviews will be slow, very slow and very argumentative
  • Team will be having arguments over things that don't exist yet: formatting and naming standards, standard patterns
  • Team will be cross training each other
  • Team will eventually converge on those things and reviews will smooth out

Recap - What is quality code?

  • Satisfies customer requirements
  • Easy to change
    • Defining feature of software compared to hardware

Recap - Get feedback faster

  • Automate builds
  • Automate deployments

Recap - Easy to change

  • Add tests
  • Do code reviews

Thanks, Any Questions?