Cleaning up code smells: Venkat Subramaniam @ Chennai

Posted on April 24th, 2011 in Agile by siddharta || 7 Comments

Today, Venkat Subramaniam gave a hilariously funny talk on cleaning up code smells. Here are some of my notes from the talk.

Cleaning up code smells

Why do we need good quality code?

  • Agile is about responding to feedback, which requires making a lot of changes to the code
  • If the code is not in good shape it gets hard to read, maintain and modify easily

What are code smells?

  • Code smells are poor quality code that cause a stink, a sense that something in wrong in the code
  • Smell is more instinctive than a checklist, you just “know” when you see smelly code
  • If you dont get rid of the smell, then after a while you get used to it and stop noticing it
  • You can write smelly code in any language: For every foolproof language there is a smarter fool :)
  • We dont even realise it that we are writing smelly code, it often takes an outsider to point them out to us
    • Sidenote: When you wan’t to criticise someones approach, dont say “Thats stupid”, say “Wow, that’s interesting…. but here is a better way”

Duplicated code

  • Can cause the same bug in many parts of the codebase
  • A guy in hyderabad: Every 2 months we fix the same bugs
  • Venkat: Did you remove the duplication?
  • Hyderabad guy: You know, thats a really good idea

Unnecessary complexity

  • Programmers are fundamentally happy to deal with complexity
  • Fear nothing more than complexity

Exception handling

  • Q: Whats worse than an empty exception catch block?
    • try { ... } catch (Exception e) { }
  • A: An empty catch block with a comment!
    • try { ... } catch (Exception e) { // is this required? }
  • Java’s checked exceptions: good or bad?
  • If you dont handle the exception, propogate it
  • If you want to handle two exceptions, then use two catch blocks, not a single one with an if condition

Switch statements & conditionals on type

  • Switch statements and conditional on type can usually be replaced by polymorphism

Long methods

  • You cant see the whole method on the screen
  • Usually means a single method with multiple responsibilities
  • Difficult to debug
  • Untestable
  • Hard to reuse -> leads to duplication as programmer copy-paste select parts of the method elsewhere
  • Conditional complexity -> cognitive overload
  • Method length: level of abstraction is more important than number of lines

Compose method pattern

  • All statements in the method at the same level of abstraction

Useless comments

  • Make the code self documenting
  • Document why, not how
  • Comments on method signatures that just repeat the signature
  • Comments that just repeat the code
    • i += 1 // increment
  • Comments in long methods that explain the different things the method does
    • Extract the sections into smaller methods & delete the comments
  • IDE vomit: Placeholder commets autogenerated by IDE
  • Poor comments often caused by TDD*
    • * (Threat driven development) – You will write comments for method signatures, long methods etc
  • Comment in production code:
    • // God help me, I have no idea what this means

Variable names

  • Find names that convey intention
  • Dont use single letter names
  • But also don’t use massively long names

Inheritance

  • Inheritance is abused more often than used
  • Composition is usually better than inheritance
  • Use inheritance for is-a relations that satisfy the Liskov Substitution principle
  • Don’t use inheritance to reuse some methods
  • To reuse a method, delegate is a better option

Viscous languages

  • Languages where it is easier to do the wrong thing than the right thing

Worst code smells

  • Long classes
  • Duplicated code
  • Unused methods
  • Unnecessary casting
  • Overuse of design patterns

Deodorising code

  • Code reviews!
    • But soon after writing it
    • And in increments
    • Also review test cases
  • Pairing is an option
    • But keep switching pairs, otherwise you get used to your smells and stop noticing them
    • Switch pairs once or twice a day

Some design principles

Some book references

Q&A

  • On using code quality tools like PMD: Such tools are great because they catch straightforward problems so that the code review can focus on higher level design principles
  • On Tools tied to IDE: People dont run them. Run the tools automated on the background (or on CI)
  • Do you need refactoring in dynamic languages: Dynamic languages don’t have many automated refactoring tools, but programmers still refactor manually
  • On design patterns for dynamic languages: Each language has its own patterns and idioms. Eg: execute around method pattern from smalltalk
  • On knowing multiple languages
    • You know multiple paradigms, multiple idioms and multiple ways of approaching a problem
    • Idioms learnt in one language and applied in others
    • You also know the perils with different approaches
  • On programming language trends: Lots of interest in functional programming, mobile device programming
  • On writing books: Think about topics over a long period of time, give lots of talks on the topic, which crystallises ideas. By the time the book is started, all the ideas are in place and the book in done in 2 weeks time
  • On thinking: Thinking is good, but you also need to look at critical thinking, that is thinking about how you think (double loop learning?)
  • On learning: Work with other people, meet and discuss in user groups. You cannot learn everything, but look to decrease the things that “you dont know that you dont know” and make them things “you know you dont know”

Doing Distributed Agile?

Share and collaborate with distributed teams with our electronic agile board tools. Get all the benefits of electronic tools without sacrificing the benefits of physical boards. Supports Scrum taskboards, Kanban boards and user story maps. Check it out!

7 Responses to “Cleaning up code smells: Venkat Subramaniam @ Chennai”

  1. Easa Says:

    Nice composition!

  2. siddharta Says:

    Thanks!

  3. ShriKant Vashishtha Says:

    Nice summary. It helps people who couldn’t join the talk.

  4. Andrew Says:

    Covered most points in the event.. Thanks

  5. Igor Popov Says:

    Great work!!! Very useful… I’ll recommend this to all my work mates…

  6. Rathinavel Says:

    Thanks Sid, well captured notes, this really helps to recall almost the entire session.

    The worst in exception handling is:

    try { … } catch (Exception e) { return null; }

  7. Magnus Skog Says:

    Hi!

    That pretty much summons up all the things I see every day and that I know has to be changed. It also makes me realize that it’s a constant struggle. Nothing will ever be perfect, but it can always get a little bit better.

    Kaizen!

    Cheers
    /Magnus

Leave a Reply