Can you find the 5 smells in "I reviewed your code last night and deleted all of it since it's crap"
Let's make this a short one.
I talked to lots of people about this statement and after lots of strange looks I got lots of comments about the behaviour. Especially about the "since it's crap". This is pretty obvious a very strong opinion about something.
But hey! Maybe the code works, passed acceptance quality gates, the client user interface looks pretty good that uses this code piece and it is deployed in production. And maybe the company is making loads of money $$$ with it.
Making lots of money (from https://www.flickr.com/photos/16210667@N02/12134787043/ )
Making lots of money.. Is my code still crap??
Ok... Let me slow down.
How did this came about?
The statement happened during the Daily Scrum after one of the devs finished talking about what he did. Another developer jumped in and finished this sentence with:
“I reviewed your code last night and deleted all of it since it's crap”
How many smells can you find in that statement and behavior?
I found 5.
1. "I reviewed"
What? 1 single person is the code gatekeeper and making sure that code is "right"??
2. "Your Code"
Is this your code, my code and every developer has their code area?
Can we talk about "Collective Code ownership" and the advantages about it?
See Martin Fowler on the topic "Collective Code Ownership"
3. "Last night"
Someone is working late nights?
Why is that? Is he not co-located or working different work hours?
How is that affecting team work?
Is this an incarnation of the "Hotel Room Rule"? The "Hotel Room Rule" as described by me on my private blog.
A code review where someone just deletes code?
What about giving feedback in a constructive way?
Can you provide feedback and avoid this in the future?
If you are a team, focus on how we can improve together. How can we guide ourselves in the direction of Clean Code. Why CleanCode in the 1st place?
More links to giving feedback in my talk about "Agile and Software Craftmanship"
5. "It's crap"
What is crap about it?
The overall class design?
The infrastructure usage? What is it?
Is this your opinion? Can we do a team code review about this?
Some tips to avoid this situation:
- Schedule regular team code reviews (we call this DevExchanges and do that every 30 minutes after lunch).
These sessions help to share knowledge and "how do we do things here" (every team is different). Did you ever have a discussion about "tabs versus whitespaces" or "member variables with a leading underscore _"?
- Use Merge Pull Requests and the comment feature to discuss code issues
Github, Gitlab or others provide great tooling around reviewing and discussing code
- Provide lots of context in commit statements
Context is important to know what problem are we trying to solve with the specific code. Is this a refactoring? Fix a bug? Why? Why? Why?
- Give feedback by asking questions. Instead of "this is crap" say "Can we improve this?"
It's very hard to ask a question and be negative in it... Not impossible though
- Use NVC to communicate, "I see", I observe", "I realize" are great statements to start with.
"I see these lines of code are crap." is not a good one;-) Non Violent Communication on Wikipedia
- Try mobprogramming or pair programming
BTW: we made quite good experiences with remote people as well when we did Dev Exchanges distributed as described by Emilija on Distributed Development.
Important to remember. Grow as a team and
"The doing is more important than the result."
Question to you:
What do you do if you browse the codebase and you see something smelly?
Put it on a list? Mark the code? Add a TODO? Send an email? Approach someone? Wait for the retrospective? Call the lead dev?