Skip to content

Instantly share code, notes, and snippets.

@monfresh

monfresh/caseflow_code_quality.md

Last active March 29, 2023 17:42
Show Gist options
  • Star 1 You must be signed in to star a gist
  • Fork 1 You must be signed in to fork a gist
  • Save monfresh/085fd396eb177c37cdb146c56166b7f3 to your computer and use it in GitHub Desktop.
Save monfresh/085fd396eb177c37cdb146c56166b7f3 to your computer and use it in GitHub Desktop.
Download ZIP
Caseflow code quality talk
Raw
caseflow_code_quality.md

Wisdom Wednesday on Code Quality, March 6, 2019

Moncef's Background:

  • Developed interest and focus on code quality and refactoring over past 4 years

  • Helped start 18F Development Guide

  • Spent lots of time reading and learning

Recommended courses, books, tutorials, newsletters

Problems

  • Onboarding is slow and different for each person
  • Different teams use different design patterns
  • High churn in files with high complexity

Recommended articles and talks on churn, complexity, code quality

Complicated code that rarely changes appears in the upper left quadrant of this graph. We abhor complication, but if the code never changes, it's not costing us money. Pretend the code is a cabinet overstuffed with teetering Tupperware: just quietly press the door closed and sneak away. Ignore code in this quadrant until it starts to churn.

Simple code that changes a lot falls into the lower right quadrant. If code is simple enough (think configuration file) it will be cheap to change. Change this code as often as necessary, as long as it remains simple.

The lower left quadrant contains things that aren't very complicated and that don't change much, so this code is already cost-effective and can also be ignored.

The top right quadrant reflects complicated code that changes often. By definition, code like this will be hard to understand and difficult to change. We'd prefer this quadrant to be empty, so code that slithers into it should be refactored right back out of it.

If important classes in your domain change often, get bigger every time they change, and are accumulating conditionals, stop adding to them right now. Use every new request as an opportunity to do a bit of design. Implement the change using small, well-designed classes that collaborate with the existing object.

A 5,000 line class exerts a gravitational pull that makes it hard to imagine creating a set of 10 line helper classes to meet a new requirement. Make new classes anyway. The way to get the outliers back on the green line where they belong is to resist putting more code in objects that are already too large. Make small objects, and over time, the big ones will disappear.

Code is read many more times than it is written. Writing code costs something, but over time the cost of reading is often higher. Anyone who ever looks at a piece of code has to invest brain-power into figuring out what it does. [...] It follows that you can reduce overall costs by optimizing code for reading rather than writing.

Here's where the concept of half-life matters. You can lower your costs by reducing the half-life of your least stable code.

The parts of your applications that change the most also cost the most. These dynamic parts are often fundamental to your business. They are the result of lengthy, evolutionary development, and suffer from never ending churn. You depend on them the most, yet they are hard to understand, and only getting worse.

Vast other swaths of your application are likely equally unpleasant, but relatively stable. This stability essentially makes them free. It's not ugly code that costs money--it's change. Ugly code just exacerbates costs.

Time is in appallingly short supply. The good news, however, is that you're not obligated to fix things that aren't costing you money. The most efficient way to improve long-lived applications is to focus on the code that churns--this is where your efforts most pay off. For maximum effect, commit to crafting solutions that are both frugal and easily replaceable.

Script to measure churn in any Git repo

https://github.com/garybernhardt/dotfiles/blob/master/bin/git-churn

These files have changed more than 100 times in the Caseflow repo:

 102 client/app/queue/QueueApp.jsx
 106 spec/feature/intake/appeal_spec.rb
 107 app/models/form8.rb
 107 client/app/queue/utils.js
 110 app/models/hearing.rb
 110 app/models/request_issue.rb
 116 app/controllers/certifications_controller.rb
 118 spec/feature/intake/supplemental_claim_spec.rb
 123 client/app/containers/EstablishClaimPage/EstablishClaim.jsx
 129 spec/rails_helper.rb
 135 app/models/user.rb
 138 app/controllers/application_controller.rb
 140 spec/feature/intake/higher_level_review_spec.rb
 153 spec/feature/establish_claim_spec.rb
 166 lib/fakes/appeal_repository.rb
 171 app/controllers/tasks_controller.rb
 195 spec/models/appeal_spec.rb
 207 app/services/appeal_repository.rb
 223 app/models/task.rb
 380 app/models/appeal.rb

The ones at the bottom are also the most complex.

Solutions

Invest in Onboarding:

From The Effective Engineer (highly recommended book):

Invest in onboarding and mentoring. The more quickly you can ramp up new team members, the more effective your team will be. [...] Training a new engineer for an hour or two a day during her first month generates much more organizational impact than spending those same hours working on the product. Moreover, the initial time investment to create onboarding resources continues to pay dividends with each additional team member.

Examples:

Agree on best practices then document them

Examples:

Recommended best practices

Action items

  • Moncef will set up Code Climate
  • Moncef will set up a meeting for engineers to discuss and document the practices we want to adopt consistently going forward.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment