Can you find the 5 smells in “I reviewed your code last night and deleted all of it since it’s crap”

from on 24.09.2015
5
Menschen sträuben sich gegen Änderungen - und damit gegen Innovationen.

Ok…
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.. 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.

4. “Deleted”
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 style?
The complexity?
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
    http://mobprogramming.org/

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?

Write a comment Denniz Cancel

Your e-mail address will not be published or shared with third parties. Fields marked with * are required.

5 comments for “Can you find the 5 smells in “I reviewed your code last night and deleted all of it since it’s crap”

  1. Denniz

    >> Making lots of money.. Is my code still crap?

    I think this statement adds an interesting perspective.
    If we make a lot of money with that code and we never need to change it, why bother?
    Code coverage is another important aspect of code quality to talk about

    Thanks anyway

    1. Bachi

      >> Making lots of money.. Is my code still crap?

      > I think this statement adds an interesting perspective.
      > If we make a lot of money with that code and we never need to change it, why bother?

      Because doing nothing will give opportunities to eager and new players. Think Nokia.

        1. Dom

          "If It Ain’t Broke, Don’t Fix It"

          If code works, has been stable for a longer period of time, and there is no reason to change it in the near future (i.e., there is no upcoming change affecting the code), why risking to break something?

          See also corresponding reengineering pattern in http://scg.unibe.ch/download/oorp/OORP.pdf

          Of course, some people might argue that reengineering legacy code and working on a newly developed system are not the same tasks. Then again, when does code become legacy? When it's deployed in production? When the developer left?