Code Smell and Heuristics – What is a Code Smell? – Flag Arguments

View Github Repository Open presentation in a new window

nkhalasi

See all presentation from nkhalasi

Code Smell and Heuristics – What is a Code Smell? – Flag Arguments

0 0

code_smell_heuristics

Presentation on code smells and heuristics

On Github nkhalasi / code_smell_heuristics

Code Smell and Heuristics

Summarized from "Clean Code" by Robert Martin

What is a Code Smell?

Have you ever wondered that your code is

Technically correct

Does function as expected

BUT

You get a sense of deeper problem

Indications of violation of fundamental design principles

Your development has slowed down

Or there is increased risk of bugs and failures

Inappropriate Information

Hold information in the right system

  • Issue/Defect information in issue tracking system
  • Change histories, author information, etc in source code control system
  • Record technical notes about code and design

Obselete Comments

Old, irrelevant and incorrect

Floating islands of irrelevance and misdirection in the code

Either update or remove obselete comments

Redundant Comments

Comments should say what the code cannot say for itself

Don't 
i++; //increment i
or 
/**
 * @param sellRequest
 * @return
 * @throws ManagedComponentException
 */
public SellResponse beginSellItem(SellRequest sellRequest)
		throws ManagedComponentException

Poorly Written Comments

A comment worth writing is worth writing well

Make sure it is the best comment

Use proper grammar and punctuation

Don't state the obvious

Commented out code

Rots and becomes less and less irrelevant with each day

Delete it. Source control system has it for reference.

Calls functions and uses variables that are no longer valid.

One Click Build

Trivialize building a project

Automate multi-steps and simplify

Explore new age build tools - SBT, Gradle, etc.

Does your build tool allow easy extensions?

One Click Testing

Make testing less painful

Running tests should be quick, easy and obvious

Functions With Too Many Arguments

No Arguments is best

Three is the limit

Time to refactor your design and abstractions when you need more arguments

Output Arguments

They are counterintuitive

Arguments are for inputs

Update state of the object being actioned

public void appendFooter(StringBuffer report)
vs 
report.appendFooter();

Flag Arguments

Strong smell and avoid them

Clear indication that your function does more than one thing

Go back and revise your understanding of SRP

def chosen_ident(choice_flag, item, match):
    assert choice_flag in (action.ASIS, action.APPLY)
    if choice_flag is action.ASIS:
        return (item.artist, item.title)
    elif choice_flag is action.APPLY:
        return (match.info.artist, match.info.title)
class _IdentChooser:
    pass

class AsisIdent(_IdentChooser):
    def __call__(self, ident):
        return (ident.artist, ident.title)

class ApplyIdent(_IdentChooser):
    def __call__(self, ident):
    	assert ident.hasattr('info') is True
        return (ident.info.artist, ident.info.title)

class NullChooser(_Chooser):
    def __call__(self, ident=None):
        return ()

choosers = {action.ASIS: AsisChooser, action.APPLY: ApplyChooser, None: NullChooser, '': NullChooser}

def chosen_ident(choice_flag, item):
    return choosers[choice_flag](item)

Dead Function

it just rots and misguides

Just discard

Obvious Behavior Not Implemented

Follow "The Principle of Least Surprise"

Not implementing the obvious results into

  • Confusions for readers of code
  • Loss of trust and thereby more efforts to read and understand the code

e.g. getDayOfWeek() should return Monday, Tuesday, etc and not 1, 2, 3, etc.

Incorrect Behavior at the Boundaries

Don't rely on your intuition

Write tests for every boundary condition

Duplication

represents a missed opportunity for abstraction

Always follow DRY principle

Abstraction increases vocabulary of your design

Improves reusability, increases coding speed and reduces error

Replace switch/case or if/else with polymorphism

Similar looking code? Does TEMPLATE METHOD or STRATEGY Pattern apply?

Code At Wrong Level of Abstraction

Seperate concepts at different levels and place them in different containers

Seperation should be complete. Don't mix lower and higher level concepts.

Identify the misplaced abstraction in 
public interface Stack {
    Object pop() throws EmptyException;
    void push(Object o) throws FullException;
    double percentFull();
    class EmptyException extends Exception {}
    class FullException extends Exception {}
}
percentFull() is applicable to BoundedStack but may not be applicable to some other implementation of Stack

Base Classes Depending on Their Derivatives

THE END

By Naresh Khalasi