How does our code become unmanageable? A practical example
- fast-forward demo through the life of an (apparently) trivial function
Hello... My name is Carlo,
and I have been writing Legacy Code for 15 years...
Can you have a look?
How does our code become unmanageable? A practical example
- fast-forward demo through the life of an (apparently) trivial function
Write an expense report tool that
- runs in a folder where employee expense excel (csv for now) are stored
- a file for each month (e.g. 01.csv for January)
10/01/2015, 10.50
11/01/2015, 8.50
12/01/2015, 5.50
15/01/2015, 8.50
- compute the monthly total and produce <>-report.txt
Month January
Expenses: 4
Amount: 33
Version 1... implemented in 15 minutes! :-)
not good but not so bad
- add a type of expense
- skip an header row
Date, Type, Amount
10/01/2015, Train, 10.50
- produce an html report
- compute expenses per category
- detect if the expenses are more than threshold
another 15 minutes - I am a 10x-programmer!
120 Lines, and already unmaintainable
- code-writing time -> decreases
- application-ready time -> never done
- time needed for bug fixes and new features -> increases
The original developer has left the company and your boss asks you:
Can you simply automatically do this for all employees? and fix the case where the input file is incorrect and fix the rounding errors And keep two intermediate totals for cash and credit card expenses?
And integrate with the acccounting application?
#:-(
Well... it will take N weeks just to understand and fix the bugs in the original code...
Which become months with regression testing
the "deadly sins" of development
- cut & paste,
- optimization lust
- haste
- false savings
- naming avarice
- trial and error wrath
- my code is perfect pride
- making it right is too hard - discouragement
Lab
- write down 3 problems with the expense report code
- try to keep track of how much time you waste because of bad code
- just tick a mark every time it happens
- Symptoms of Rotten Design
Clean Code, Design Principles and Lean to the rescue
- improving our code
- improving our design
- practice, practice, practice and continuous / daily improvement (Kaizen)
- Ideas -> 3 post it
-
Ignaz Semmelweis
-
He championed washing hands before childbirth and surgery
-
he was obstracized by the medical community!
- it can't be that simple...
- we just don't have time...
-
And now?
-
It cannot solve all development problems...
-
But it can make them way more tractable!
Once we have got the basics covered, then we will need to understand the Software Dynamics
- vs the nature (and Laws) of Software
Take them into account => Design Principles
Basically, Common Sense applied to software design
Treat your code like your kitchen C.B., about 2013
It takes a Deliberate approach and constant effort
To complicate is easy, to simplify is hard To complicate, just add, everyone is able to complicate Few are able to simplify Bruno Munari
If I don't practice for a day, I notice If I don't practice for two days, my orchestra notices If I don't practice for three days, the public notice Claudio Abbado
- practice, practice, practice and continuous / daily improvement (Kaizen)
- deliberate practice -> iterate small skills until >90% perfect
Code Katas
- reading code vs writing code
- what is a good name?
- same but different: the importance of conventions
- be meaningful
- aside: commit messages
A little experiment
Write down:
- what the software does
- how long it took to understand it
- which bugs you can find in the code
- how long it took to find them
Group A: go to http://plnkr.co/edit/dQldXF
Group B: go to http://plnkr.co/edit/zPXf70?
What is written without effort is in general read without pleasure.
Samuel Johnson
Most code is written once, but read
- every time you need to fix a bug
- to add new features
- by other developers
- including your future self
- Ideas?
- nonsense
- honest
- honest & complete
- does the right thing
- intent
- domain abstraction
http://llewellynfalco.blogspot.it/p/infographics.html
- good code shouldn't almost require naming comments
How do you read this code:
float tempC;
float farTemp
float celsiusTemperature;
float farenheitTemperature;
Lab 1: write down a function for computing the amount of days between two dates
- jot it down
- check that it works
- review the naming of each variable and function
- check that it works
- review the naming - again
- discuss it with your neighbour
Lab 2: refactor the first example
- avoid mental mapping
formatMessage("File not found",2);
formatMessage("File not available",1);
formatMessage("File loaded",3);
-
Mental energy is finite
- attention over time
- amount of information: 7 +/- 2
-
do not waste it in useless mappings
Everything is like code
- configuration files
- infrastructure
- scripts And
- commit log messages
- a message to your future self (and team mates)
git commit -m "bug fix"
- a message to your future self (and team mates)
- https://en.wikipedia.org/wiki/Leap_year# Algorithm
- http://blog.speziale.it/post/Scorporo-dei-prezzi-ivati-problema-del-e2809ccentesimo-pazzoe2809d.aspx
- making code readable
- making code diff-friendly & commit-friendly
- making code modification-friendly
What does this code do? http://plnkr.co/edit/mZtyDG?p=info
- the IDE does most of that for you
- share a team guideline
- possibly, a shared style template
- diffs work mainly line by line
- each line should have a different reason / time to change
<div>Threshold <input type="number" style="color : red" min ="5"
max = "57" class ="outline"
></div>vs
<div>
Threshold
<input type="number" min ="5" max = "57"
style="color : red" class ="outline"
>
</div>- this makes thing also merge-friendly
- A modification per line
Refactor this
- single responsibility
- separing inputs from outputs
- if you have to do 3 things, make 4 functions
- primitives and orchestrators
See the gen() function again
Each function should do 1 thing
Or even better, have a single responsibility
- and reason to change
Ask yourself questions...
- What?
- Who?
- When?
- Why?
- Where?
And put the answer in different sub-functions
- make inputs clear
- limit / avoid output parameters
If your function needs to perform a non-trivial task:
- import data, transform it and store it in the DB
Instead of
readData(){
file.open();
while(..)
{
line = readLine();
obj = trasformLine(line);
saveInDB(obj);
}
}what's better?
importData(){
data = readData();
obj = transformData (data);
saveInDB(obj);
}
- a function for each step
- a function to call the steps
-
Primitives: small, focused, typically use-case independent
-
Orchestrators: implement use-cases by combining primitives
-
rinse and repeat over multiple levels of abstraction
-
benefits:
- more reusable
- easier to test
- Parse the Meteo Data file and compute the weekly min and max temperature
- Single Responsibility Principle
- collaborating with other classes
- composition vs inheritance (and the Open/Closed principle)
- Dependency Injection
- interfaces and the importante of Contracts
Have you ever seen your grandmother put dirty clothes in the fridge?
Or biscuits in the vegetable box?
So, why to we do this all the time in our code?
Responsibility == reason to change
A class should do one thing
- and have a single reason to change
Consequences:
- classes should be small
- classes should be focused
- classes need to collaborate to perform complex tasks
- Take the "ugly" code or any other code example
- Paste it in word / Google Docs
- Outline in different colors the various responsibilities
-
We need a way of making collaboration easier
-
With Dependency Injection
- separate creation of classes from linking instances
- create A
- create B
- something else passes B to A
-
You do not need a framework for that...
- Inheritance is the strongest link between classes
- useful with caution
- combine parts
- a derived class becomes the composition of a base behaviour + additional custom behaviour
Open for extension, Closed for Modification
- esplicit vs implicit
- Decoupling changes and detecting regressions
- separate clean parts from dirty code
-
Design the classes for the additional requirements for the expense report
-
Can you simply automatically do this for all employees?
-
and fix the case where the input file is incorrect
-
keep two intermediate totals for cash and credit card expenses?
-
integrate with the acccounting application
- avoid statics
- testable code vs good design
- avoid statics
- decouple code
Things that make code testable
- clear interfaces
- small classes / functions
- decoupled
- composition
Things that make code well-designed, easy to evolve
- clear interfaces
- small classes / functions
- decoupled
- composition
- one task - one statement
- make return values visible
- logging
BufferedReader reader = new BufferedReader(new FileReader(fileName));
String line = reader.readLine();
while(line != null)
{
String [] lineElements = line.split(",");
forecasts.add(new MeteoForecast(lineElements[1],
formatter.parse(lineElements[0]),
Integer.parseInt(lineElements[2]),Integer.parseInt(lineElements[3].trim()), lineElements[4])
);It contains a mistake - go debug it! ./labs/lab-debug-01
The forecast.add(... line both
- has multiple responsibilities
- manipulates multiple values
So split it!
Even one statement -> multiple lines,
- where the language supports it
Having temporary variables for return values
- makes debugging easier
- inspecting the value during construction
- inspecting the value before returning
- even modifying it in some debuggers
- makes type errors more visible
Also modern debuggers are able to display the value inline
- make log messages understandable
- avoid mental mapping
- make them easy to search
Refactor the example
Your code should read like a terse prose of simple statements
- this also helps with debugging!
- from bad code to good code
- in steps
- learn your IDE refactoring tools
- The "Boy Scout Rule"
- Why we need unit tests?
- Go back to the "ugly" example
Incremental transformation
-
Each step should not change the functional properties of the system
-
and improve the non-functional ones
-
separate adding features from refactoring
- don't do both in the same step
- if you have to pick one: Find all references
- Refactor / Rename
- Extract Method
- Extract Interface
- http://refactoring.com/
Leave the campsite a little better than you found it
Every time you touch some code, leave it a little better
The power of compounding many small changes in the same direction
- 1% time
- be confident that we can change the code with very low risk
- simple refactorings are performed at the AST level in the IDE
- renaming
- extracting method
-
Refactor the initial Expense Report example
-
name things
-
extract sub-methods
-
create classes
-
extract interfaces
So what did we just do? Understand the principles
- the relationship between quality and productivity
- the need for a continuous chain of small, safe steps of design & implementation
Traditionally, Quality is seen as an alternative to raw development Speed
- this is partly true only in the short term
Four quadrants:
-
high quality, high productivity -> tends to further improve
-
high quality, low productivity -> tends not to improve, and go to
-
low quality, low productivity -> tends to get worse
-
low quality, high productivity -> tends to go to the previous one
-
Productivity curves at different quality ratio
-
Faster small steps beat bigger steps
-
also easier to parallelize
-
The smaller the better
- you need to be able to check that everything works
- run frequently
- test frequently
- Making things easier to change
- This does not mean that you do not have a vision
- Plan the overall Path
- but execute a step at a time
Define the main structure Split in sub-tastks with post-its Discuss the optimal order Introduce mock / support steps
- Elefant Carpaccio
- how to do everything incrementally
- Separation of Concerns in practice: ask yourself questions!
- incremental implementation: in-application Mocks & the Walking Skeleton approach
- how to keep track of what you do and what is missing
- how to manage incremental commits
- You can do everything incrementally
- decouple release from deployment
- branch by abstraction
- do both
- expand-contract
- what if instead I only do X?
- A & B -> A then B
- like in the tests
- entire application / workflow structure
- made of empty (or logging-only) components
- incrementally filled-in
- also useful for testing
- Write it down
- comment it with temporary comments
- code it!
-
Each commit should start from a stable state and lead to a stable but more complete state
-
Push vs commit in DVCS
http://continuousdelivery.com/
Define the main structure Split in sub-tastks with post-its Discuss the optimal order Introduce mock / support steps
- the feedback loop
- splitting the problem
- getting more feedback
- getting feedback more frequently
- model the problem
- avoid trial and error, but if you need it, do it fast
When it does not work...
And we do not know why...
- Idea --> Change --> Observation --> Evaluation
Learning
- limit in the amount of changes
- time required
- to effect change
- to observe the result
- information collected after the change
- split the problem
- make it smaller
- make it independent
- adding more log / monitoring
- adding higher quality log info
- adding dedicated code / validation logic
- better tools (inspectors...)
- make the loop faster
- e.g. jrebel
- gulp serve
- livereload
- Hypothesis
- Which test do I need?
- write it down
- does the result conforms to the expectation?
- reduce uncertainty
Or at least do it fast
- Kathy Sierra
- https://www.youtube.com/watch?v=FKTxC9pl-WM