code smell

a code smell is a surface indication that usually corresponds to a deeper problem

– Martin Flower (Fowler, 2006)

clean clode violations as code smells

• long method
• long parameter list
• naming
  • notation in names
  • inconsistent names
  • uncommunicative names
• comments
• large class
  • possibly do more than one thing
• a function / class does more than one thing

some code smells

• duplicated code
  • Don’t Repeat Yourself! (a.k.a., DRY principle) (Venners, 2003)
• speculative generality
  • do not generalize the code to solve a potential future problem
  • You aren’t gonna need it (YAGNI)
  • focus on today’s problem
• dead code
  • e.g., a function that is never called
  • editors denote it
  • in Go unused variable is not a warning, but an error
• temporary field
  • “Watch out for objects that contain a lot of optional or unnecessary fields. If you’re passing an object as a parameter to a method, make sure that you’re using all of it and not cherry-picking single fields.” (Atwood, 2006)

conditional complexity

if a and b:
    do_something()

if a or b:
    do_something()

if not (a or (b and not c) and (d or not f)):
    do_something()

conditional complexity

if is_pressure_low() and is_temperature_high():
    do_something()

if is_pressure_low() or is_temperature_high():
    do_something()

if not (
    is_pressure_low()
    or (is_temperature_high() and not is_humidity_low())
    and (is_fall() or not is_raining())
):
    do_something()

class-based smells: alternative classes with different interfaces

If two classes are similar on the inside, but different on the outside, perhaps they can be modified to share a common interface (Atwood, 2006).

class-based smells: data class???

class-based smells: data clumps

If you always see the same data hanging around together, maybe it belongs together. Consider rolling the related data up into a larger class (Atwood, 2006).

class-based smells: refused bequest

If you inherit from a class, but never use any of the inherited functionality, should you really be using inheritance? (Atwood, 2006)

class-based smells: indecent exposure

class-based smells: feature envy

Methods that make extensive use of another class may belong in another class. Consider moving this method to the class it is so envious of.

– Jeff Atwood (Atwood, 2006)

more code smells

this section is based on the book Clean Code (chapter 17) by Robert C. Martin (Martin, 2009)

with own examples

comment related smells

1. obsolete comment

version n-1 (OOP)

# increase class attribute
def increase(self, by):
    self.foo += by

version n (FP)

# increase class attribute
def increase(what, by):
    return what + by

comment related smells

2. redundant comment

# creates an empty dataframe
def create_empty_dataframe(start_week, end_week):

comment related smells

3. commented-out code

def increase(what, by):
    # print(what, by)
    return what + by

not needed, just remove it

class Something:
    foo = 0
    
    def increase(self, by):
        self.foo += by
    
    def decrease(self, by):
        self.foo -= by
    
    # def mutiply(self, by):
    #     self.foo *= by

magic numbers

magic number is an unexplained constant in the code

def calculate_circle_area(r: float) -> float:
    return r * r * 3.141592

PI = 3.141592

def calculate_circle_area(r: float) -> float:
    return r * r * PI

import math


def calculate_circle_area(r: float) -> float:
    return r * r * math.pi

encapsulate boundary conditions

if level + 1 < length:
    do_somthing(foo, bar, level + 1)

next_level = level + 1
if next_level < length:
    do_somthing(foo, bar, next_level)

also increases consistency, the condition needs to be adjusted in one place

denoting blocks

for (i = 0; i < 10; i++) {
    console.log(i);
}

for (i = 0; i < 10; i++)
    console.log(i);

var a = 0;
for (i = 0; i < 10; i++)
    a++;
    console.log(i);

for i in range(10):
    print(i)

a = 0
for i in range(10):
    a += 1
    print(i)

package main
 
import (
    "fmt"
)
 
func main() {
    for i:=0; i<10; i++ {
        fmt.Println(i)
    }
}

fn main() {
    for i in 0..9 {
        println!("{}", i);
    }
}

what could go wrong?

if ((err = ReadyHash(&SSLHashSHA1, &hashCtx)) != 0)
    goto fail;
if ((err = SSLHashSHA1.update(&hashCtx, &clientRandom)) != 0)
    goto fail;
if ((err = SSLHashSHA1.update(&hashCtx, &serverRandom)) != 0)
    goto fail;
if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
    goto fail;
    goto fail;
if ((err = SSLHashSHA1.final(&hashCtx, &hashOut)) != 0)
    goto fail;

fail:
    SSLFreeBuffer(&signedHashes);
    SSLFreeBuffer(&hashCtx);
    return err;

how to measure code quality?

it is hard to objectively measure the quality of code

Halstead metrics

Halstead’s goal was to identify measurable properties of software, and the relations between them (Radon documentation, n.d.).

cyclomatic comlexity

• developed by Thomas J. McCabe in 1976
• quantitative measure of the number of linearly independent paths through the source code
• computed using the control-flow graph of the program

defined as:

$$ M = E - N + 2P $$

cyclomatic comlexity – example

def calculate_progress(
    finished: int,
    total: int,
    as_percentage: bool
) -> float:
    progress = finished / total

    if as_percentage:
        return progress * 100
    else:
        return progress

activity diagram

control flow

cyclomatic comlexity – 2nd example

def calculate_progress(
    finished: int,
    total: int,
    as_percentage: bool,
    foo: bool
) -> float:
    progress = finished / total

    if as_percentage and foo:
        return progress * 100
    else:
        return progress

activity diagram

control flow

Python statements’ effects on cyclomatic complexity

interpretation of cyclomatic complexity – Radon

maintainability index

original (Coleman et al., 1994):
$$ MI = 171 - 5.2 \ln{V} - 0.23G - 16.2\ln{L} $$

Visual Studio derivate:
$$ MI = max\left[0,100 \frac{171 - 5.2 \ln{V} - 0.23G - 16.2\ln{L}}{171}\right] $$

maintainability index – example

def calculate_progress(finished: int, total: int, as_percentage: bool) -> float:
    progress = finished / total

    if as_percentage:
        return progress * 100
    else:
        return progress


def calculate_progress_2(
    finished: int, total: int, as_percentage: bool, foo: bool
) -> float:
    progress = finished / total

    if as_percentage and foo:
        return progress * 100
    else:
        return progress

• maintainability index for a script containing the code above is 63.71
• calculated with Radon

go report

• gofmt: style guide
• go_vet: reports suspicious constructs (Go specific)
• ineffassign: detects ineffectual assignments in Go code
• gocyclo: cyclomatic complexity
• license: checks whether your project has a LICENSE file
• misspell: finds commonly misspelled English words

score	grade
>90
>80
>70
>60
>50
>40
<=40

Go Report Card for Gitea

code chunk permanence in a codebase

WTF per minute

references