Tweet
Tweet

Slide 1

Slide 1 text

ElixirConf US 2023 EXPLORING CODE SMELLS

Slide 2

Slide 2 text

ELAINE NAOMI WATANABE B.Sc. in Computer Engineering M.Sc. in Computer Science Senior Software Engineer @ TheRealReal twitter.com/elaine_nw speakerdeck.com/elainenaomi linkedin.com/in/elainenaomi

Slide 3

Slide 3 text

No content

Slide 4

Slide 4 text

The RealReal - Elixir Adoption Story youtube.com/watch?v=sTs_4T1ufLY

Slide 5

Slide 5 text

CODE SMELLS

Slide 6

Slide 6 text

No content

Slide 7

Slide 7 text

https://ieeexplore.ieee.org/document/6671299 2013

Slide 8

Slide 8 text

https://ieeexplore.ieee.org/document/6671299 2013

Slide 9

Slide 9 text

Do Developers Care about Code Smells? 2013

Slide 10

Slide 10 text

Do Developers Care about Code Smells? Concerns about code smells 2013

Slide 11

Slide 11 text

Do Developers Care about Code Smells? Concerns about code smells 2013

Slide 12

Slide 12 text

Do Developers Care about Code Smells? 52% Concerns about code smells 2013

Slide 13

Slide 13 text

What are their concerns about code smells?

Slide 14

Slide 14 text

Developer productivity Product evolvability Quality of end-product Self-improvement etc What are their concerns about code smells?

Slide 15

Slide 15 text

Do Developers Care about Code Smells? 52% Concerns about code smells 2013

Slide 16

Slide 16 text

Do Developers Care about Code Smells? Concerns about code smells 2013 26%

Slide 17

Slide 17 text

Lack of awareness about code smells No concerns

Slide 18

Slide 18 text

2013 2013

Slide 19

Slide 19 text

Knowledge about code smells 2013 2013

Slide 20

Slide 20 text

Knowledge about code smells 2013 2013 58%

Slide 21

Slide 21 text

#

Slide 22

Slide 22 text

TERMINOLOGY

Slide 23

Slide 23 text

https://martinfowler.com/bliki/CodeSmell.html https://en.wikipedia.org/wiki/Code_smell CODE (BAD) SMELLS Any characteristic in the source code that may indicate a deeper quality problem in the system

Slide 24

Slide 24 text

?

Slide 25

Slide 25 text

? Nope

Slide 26

Slide 26 text

They are symptoms of poor design and implementation choice. Tufano, Michele et al. "When and why your code starts to smell bad". 37th IEEE International Conference on Software Engineering (2015): 403-414.

Slide 27

Slide 27 text

https://martinfowler.com/bliki/DesignStaminaHypothesis.html design payoff line no design good design cumulative functionality time

Slide 28

Slide 28 text

Code smells may slow down development or increase the risk of bugs or failures in the future https://en.wikipedia.org/wiki/Code_smell

Slide 29

Slide 29 text

1999 22 code smells

Slide 30

Slide 30 text

2018 18+ years later 24 code smells

Slide 31

Slide 31 text

Mysterious Name Duplicated Code Long Function Long Parameter List Global Data Mutable Data Divergent Change Shotgun Surgery Feature Envy Data Clumps Primitive Obsession Repeated Switches Loops Lazy Element Speculative Generality Temporary Field Message Chains Middle Man Insider Trading Large Class Alternative Classes with Different Interfaces Data Class Refused Bequest Comments 2018

Slide 32

Slide 32 text

And how to fix code smells?

Slide 33

Slide 33 text

1999 22 code smells

Slide 34

Slide 34 text

22 code smells 72 refactoring techniques 1999

Slide 35

Slide 35 text

2018 18+ years later 24 code smells 61 refactoring techniques

Slide 36

Slide 36 text

2018 18+ years later 24 code smells 61 refactoring techniques

Slide 37

Slide 37 text

CATALOG OF REFACTORINGS Change Function Declaration Change Reference to Value Change Value to Reference Collapse Hierarchy Combine Functions into Class Combine Functions into Transform Consolidate Conditional Expression Decompose Conditional Encapsulate Collection Encapsulate Record Encapsulate Variable Extract Class Extract Function Extract Superclass Extract Variable Hide Delegate Inline Class Inline Function Inline Variable Introduce Assertion Introduce Parameter Object Introduce Special Case Move Field Move Function Move Statements into Function Move Statements to Callers Parameterize Function Preserve Whole Object Pull Up Constructor Body Pull Up Field Pull Up Method Push Down Field Push Down Method Remove Dead Code Remove Flag Argument Remove Middle Man Remove Setting Method Remove Subclass Rename Field Rename Variable Replace Command with Function Replace Conditional with Polymorphism Replace Constructor with Factory Function Replace Derived Variable with Query Replace Function with Command Replace Inline Code with Function Call Replace Loop with Pipeline Replace Nested Conditional with Guard Clauses Replace Parameter with Query Replace Primitive with Object Replace Query with Parameter Replace Subclass with Delegate Replace Superclass with Delegate Replace Temp with Query Replace Type Code with Subclasses Separate Query from Modifier Slide Statements Split Loop Split Phase Split Variable Substitute Algorithm https://refactoring.com/catalog/ 2018

Slide 38

Slide 38 text

REFACTORING

Slide 39

Slide 39 text

No content

Slide 40

Slide 40 text

No content

Slide 41

Slide 41 text

No content

Slide 42

Slide 42 text

No content

Slide 43

Slide 43 text

No content

Slide 44

Slide 44 text

No content

Slide 45

Slide 45 text

CI is failing for weeks…

Slide 46

Slide 46 text

Hard to test, hard to deliver…

Slide 47

Slide 47 text

No content

Slide 48

Slide 48 text

And, occasionally …

Slide 49

Slide 49 text

This is NOT refactoring

Slide 50

Slide 50 text

https://martinfowler.com/books/refactoring.html https://en.wikipedia.org/wiki/Code_refactoring REFACTORING A controlled technique for improving the design of an existing code base.

Slide 51

Slide 51 text

Small behavior-preserving transformations

Slide 52

Slide 52 text

Small behavior-preserving transformations *about the observable behavior

Slide 53

Slide 53 text

Identify a code smell To fix now? Apply Refactoring Run tests Yes Next No Finish

Slide 54

Slide 54 text

No content

Slide 55

Slide 55 text

defmodule Cart do # ... def exec(i, d) do i |> Enum.map(& &1.pr * &1.qt) |> Enum.sum() |> Kernel.-(d) end end

Slide 56

Slide 56 text

defmodule Cart do # ... def exec(i, d) do i |> Enum.map(& &1.pr * &1.qt) |> Enum.sum() |> Kernel.-(d) end end What are these variables?

Slide 57

Slide 57 text

Code Smell: "Mysterious Name" Also called "Bad Naming"

Slide 58

Slide 58 text

Refactoring: "Rename Variable"

Slide 59

Slide 59 text

defmodule Cart do # ... def exec(i, d) do i |> Enum.map(& &1.pr * &1.qt) |> Enum.sum() |> Kernel.-(d) end end Rename the variables

Slide 60

Slide 60 text

defmodule Cart do # ... def exec(items, cart_discount) do items |> Enum.map(& &1.pr * &1.qt) |> Enum.sum() |> Kernel.-(cart_discount) end end

Slide 61

Slide 61 text

defmodule Cart do # ... def exec(items, cart_discount) do items |> Enum.map(& &1.pr * &1.qt) |> Enum.sum() |> Kernel.-(cart_discount) end end Run tests

Slide 62

Slide 62 text

defmodule Cart do # ... def exec(items, cart_discount) do items |> Enum.map(& &1.pr * &1.qt) |> Enum.sum() |> Kernel.-(cart_discount) end end What does `exec` mean?

Slide 63

Slide 63 text

Code Smell: "Mysterious Name"

Slide 64

Slide 64 text

defmodule Cart do # ... def calculate_total(items, cart_discount) do items |> Enum.map(& &1.pr * &1.qt) |> Enum.sum() |> Kernel.-(cart_discount) end end

Slide 65

Slide 65 text

Refactoring: "Change Function Declaration"

Slide 66

Slide 66 text

defmodule Cart do # ... def calculate_total(items, cart_discount) do items |> Enum.map(& &1.pr * &1.qt) |> Enum.sum() |> Kernel.-(cart_discount) end end

Slide 67

Slide 67 text

defmodule Cart do # ... def calculate_total(items, cart_discount) do items |> Enum.map(& &1.pr * &1.qt) |> Enum.sum() |> Kernel.-(cart_discount) end end Find all old references and update them

Slide 68

Slide 68 text

defmodule Cart do # ... def calculate_total(items, cart_discount) do items |> Enum.map(& &1.pr * &1.qt) |> Enum.sum() |> Kernel.-(cart_discount) end end Run tests

Slide 69

Slide 69 text

defmodule Cart do # ... def calculate_total(items, cart_discount) do items |> Enum.map(& &1.pr * &1.qt) |> Enum.sum() |> Kernel.-(cart_discount) end end

Slide 70

Slide 70 text

Code Smell: "Mysterious Name"

Slide 71

Slide 71 text

Refactoring: "Rename Field"

Slide 72

Slide 72 text

defmodule Cart do # ... def calculate_total(items, cart_discount) do items |> Enum.map(& &1.pr * &1.qt) |> Enum.sum() |> Kernel.-(cart_discount) end end

Slide 73

Slide 73 text

defmodule Cart do # ... def calculate_total(items, cart_discount) do items |> Enum.map(& &1.pr * &1.qt) |> Enum.sum() |> Kernel.-(cart_discount) end end Skipping for now :)

Slide 74

Slide 74 text

#

Slide 75

Slide 75 text

What are we trying to achieve?

Slide 76

Slide 76 text

+ Maintainability + Extensibility + Reusability + Code readability + Performance + Clarity What are we trying to achieve?

Slide 77

Slide 77 text

#

Slide 78

Slide 78 text

Finding code smells

Slide 79

Slide 79 text

No content

Slide 80

Slide 80 text

defmodule WeatherAnalysis do def evaluate(temperature, unit) do if temperature > 30 and unit == "C" do "It's hot!" else if temperature <= 30 and temperature > 15 and unit == "C" do "It's not that hot nor that cold!" else if temperature <= 15 and unit == "C" do "It's cold." end end end end end Temperature-based analysis (Celsius only)

Slide 81

Slide 81 text

Temperature-based analysis (Celsius only) defmodule WeatherAnalysis do def evaluate(temperature, unit) do if temperature > 30 and unit == "C" do "It's hot!" else if temperature <= 30 and temperature > 15 and unit == "C" do "It's not that hot nor that cold!" else if temperature <= 15 and unit == "C" do "It's cold." end end end end end

Slide 82

Slide 82 text

defmodule WeatherAnalysis do def evaluate(temperature, unit) do if (temperature > 30 and unit == "C") or (temperature > 89.6 and unit == "F") do "It's hot!" else if (temperature <= 30 and temperature > 15 and unit == "C") or (temperature <= 89.6 and temperature > 59 and unit == "F") do "It's not that hot nor that cold!" else if (temperature <= 15 and unit == "C") or (temperature <= 59 and unit == "F") do "It's cold." end end end end end Temperature-based analysis (Celsius and Fahrenheit)

Slide 83

Slide 83 text

defmodule WeatherAnalysis do def evaluate(temperature, unit) do if (temperature > 30 and unit == "C") or (temperature > 89.6 and unit == "F") do "It's hot!" else if (temperature <= 30 and temperature > 15 and unit == "C") or (temperature <= 89.6 and temperature > 59 and unit == "F") do "It's not that hot nor that cold!" else if (temperature <= 15 and unit == "C") or (temperature <= 59 and unit == "F") do "It's cold." end end end end end Temperature-based analysis (Celsius and Fahrenheit)

Slide 84

Slide 84 text

Temperature-based analysis (Celsius and Fahrenheit) defmodule WeatherAnalysis do def evaluate(temperature, unit) do if (temperature > 30 and unit == "C") or (temperature > 89.6 and unit == "F") do "It's hot!" else if (temperature <= 30 and temperature > 15 and unit == "C") or (temperature <= 89.6 and temperature > 59 and unit == "F") do "It's not that hot nor that cold!" else if (temperature <= 15 and unit == "C") or (temperature <= 59 and unit == "F") do "It's cold." end end end end end

Slide 85

Slide 85 text

No content

Slide 86

Slide 86 text

Let's use pattern matching!

Slide 87

Slide 87 text

defmodule WeatherAnalysis do def evaluate(temperature, "C") when temperature > 30, do: "It's hot!" def evaluate(temperature, "F") when temperature > 89.6, do: "It's hot!" def evaluate(temperature, "C") when temperature <= 30 and temperature > 15, do: "It's not that hot nor that cold!" def evaluate(temperature, "F") when temperature <= 89.6 and temperature > 59, do: "It's not that hot nor that cold!" def evaluate(temperature, "C") when temperature <= 15, do: "It's cold." def evaluate(temperature, "F") when temperature <= 59, do: "It's cold." end

Slide 88

Slide 88 text

defmodule WeatherAnalysis do def evaluate(temperature, "C") when temperature > 30, do: "It's hot!" def evaluate(temperature, "F") when temperature > 89.6, do: "It's hot!" def evaluate(temperature, "C") when temperature <= 30 and temperature > 15, do: "It's not that hot nor that cold!" def evaluate(temperature, "F") when temperature <= 89.6 and temperature > 59, do: "It's not that hot nor that cold!" def evaluate(temperature, "C") when temperature <= 15, do: "It's cold." def evaluate(temperature, "F") when temperature <= 59, do: "It's cold." end

Slide 89

Slide 89 text

No content

Slide 90

Slide 90 text

In both examples: Duplicated code for Fahrenheit and Celsius Many execution paths

Slide 91

Slide 91 text

TRADITIONAL CODE SMELLS IN ELIXIR CONTEXT

Slide 92

Slide 92 text

Code Smell: "Duplicated Code"

Slide 93

Slide 93 text

Refactorings Extract Function, Slide Statements, Pull Up Method Code Smell Duplicated code

Slide 94

Slide 94 text

Refactorings Extract Function, Slide Statements, Pull Up Method DRY, Single Responsibility Principle Code Smell Duplicated code

Slide 95

Slide 95 text

#

Slide 96

Slide 96 text

defmodule WeatherAnalysis do def evaluate(temperature, "C") when temperature > 30, do: "It's hot!" def evaluate(temperature, "F") when temperature > 89.6, do: "It's hot!" def evaluate(temperature, "C") when temperature <= 30 and temperature > 15, do: "It's not that hot nor that cold!" def evaluate(temperature, "F") when temperature <= 89.6 and temperature > 59, do: "It's not that hot nor that cold!" def evaluate(temperature, "C") when temperature <= 15, do: "It's cold." def evaluate(temperature, "F") when temperature <= 59, do: "It's cold." end

Slide 97

Slide 97 text

Refactoring: Extract Function

Slide 98

Slide 98 text

defmodule WeatherAnalysis do def evaluate(temperature, "C") when temperature > 30, do: "It's hot!" def evaluate(temperature, "F") when temperature > 89.6, do: "It's hot!" def evaluate(temperature, "C") when temperature <= 30 and temperature > 15, do: "It's not that hot nor that cold!" def evaluate(temperature, "F") when temperature <= 89.6 and temperature > 59, do: "It's not that hot nor that cold!" def evaluate(temperature, "C") when temperature <= 15, do: "It's cold." def evaluate(temperature, "F") when temperature <= 59, do: "It's cold." end

Slide 99

Slide 99 text

defmodule WeatherAnalysis do def evaluate(temperature, unit) do temperature |> parse(unit) |> do_evaluate() end # ... end

Slide 100

Slide 100 text

defmodule WeatherAnalysis do def evaluate(temperature, unit) do temperature |> parse(unit) |> do_evaluate() end # ... end

Slide 101

Slide 101 text

defmodule WeatherAnalysis do # ... defp parse(temperature, "C"), do: temperature defp parse(temperature, "F"), do: (temperature - 32) * 5 / 9 # ... end

Slide 102

Slide 102 text

defmodule WeatherAnalysis do def evaluate(temperature, unit) do temperature |> parse(unit) |> do_evaluate() end # ... end

Slide 103

Slide 103 text

defmodule WeatherAnalysis do # ... defp do_evaluate(temperature) when temperature > 30, do: "It's really hot!" defp do_evaluate(temperature) when temperature <= 30 and temperature > 15, do: "It's not that hot nor that cold!" defp do_evaluate(temperature) when temperature <= 15, do: "It's cold." end

Slide 104

Slide 104 text

Code Smell: "Large Class" * Large Module

Slide 105

Slide 105 text

Refactorings Extract Class, Extra Superclass, Replace Type Code with Subclasses Code Smell Large Class

Slide 106

Slide 106 text

defmodule ShoppingCart do # Rule 1 def calculate_total(items, subscription) do # ... end # Rule 2 def calculate_shipping(zip_code, %{id: 3}), do: 0.0 def calculate_shipping(zip_code, %{id: 4}), do: 0.0 def calculate_shipping(zip_code, _) do 10.0 * Location.calculate(zip_code) end # Rule 3 def apply_discount(total, %{id: 3}), do: total * 0.9 def apply_discount(total, %{id: 4}), do: total * 0.9 def apply_discount(total, _), do: total # Rule 4 def send_message_subscription(%{id: 3}, _), do: nil def send_message_subscription(%{id: 4}, _), do: nil def send_message_subscription(subscription, user), do: Subscription.send_email_upgrade(subscription, user) # Rule 5 def print(user, order) do # ... end end

Slide 107

Slide 107 text

Rule #1 - Calculate Total defmodule ShoppingCart do # Rule 1 def calculate_total(items, subscription) do # ... end # Rule 2 def calculate_shipping(zip_code, %{id: 3}), do: 0.0 def calculate_shipping(zip_code, %{id: 4}), do: 0.0 def calculate_shipping(zip_code, _) do 10.0 * Location.calculate(zip_code) end # Rule 3 def apply_discount(total, %{id: 3}), do: total * 0.9 def apply_discount(total, %{id: 4}), do: total * 0.9 def apply_discount(total, _), do: total # Rule 4 def send_message_subscription(%{id: 3}, _), do: nil def send_message_subscription(%{id: 4}, _), do: nil def send_message_subscription(subscription, user), do: Subscription.send_email_upgrade(subscription, user) # Rule 5 def print(user, order) do # ... end end Rule #2 - Calculate Shipping Taxes Rule #3 - Apply Discount Rule #4 - Send Subscription Notification Rule #5 - Print Order

Slide 108

Slide 108 text

Hard to read Hard to explain what the module does Multiple business rules defmodule ShoppingCart do # Rule 1 def calculate_total(items, subscription) do # ... end # Rule 2 def calculate_shipping(zip_code, %{id: 3}), do: 0.0 def calculate_shipping(zip_code, %{id: 4}), do: 0.0 def calculate_shipping(zip_code, _) do 10.0 * Location.calculate(zip_code) end # Rule 3 def apply_discount(total, %{id: 3}), do: total * 0.9 def apply_discount(total, %{id: 4}), do: total * 0.9 def apply_discount(total, _), do: total # Rule 4 def send_message_subscription(%{id: 3}, _), do: nil def send_message_subscription(%{id: 4}, _), do: nil def send_message_subscription(subscription, user), do: Subscription.send_email_upgrade(subscription, user) # Rule 5 def print(user, order) do # ... end end

Slide 109

Slide 109 text

Refactoring: "Extract Class"

Slide 110

Slide 110 text

defmodule ShoppingCart do # Rule 1 def calculate_total(items, subscription) do # ... end # Rule 2 def calculate_shipping(zip_code, %{id: 3}), do: 0.0 def calculate_shipping(zip_code, %{id: 4}), do: 0.0 def calculate_shipping(zip_code, _) do 10.0 * Location.calculate(zip_code) end # Rule 3 def apply_discount(total, %{id: 3}), do: total * 0.9 def apply_discount(total, %{id: 4}), do: total * 0.9 def apply_discount(total, _), do: total # Rule 4 def send_message_subscription(%{id: 3}, _), do: nil def send_message_subscription(%{id: 4}, _), do: nil def send_message_subscription(subscription, user), do: Subscription.send_email_upgrade(subscription, user) # Rule 5 def print(user, order) do # ... end end

Slide 111

Slide 111 text

defmodule ShoppingCart do # Rule 1 def calculate_total(items, subscription) do # ... end # Rule 2 def calculate_shipping(zip_code, %{id: 3}), do: 0.0 def calculate_shipping(zip_code, %{id: 4}), do: 0.0 def calculate_shipping(zip_code, _) do 10.0 * Location.calculate(zip_code) end # Rule 3 def apply_discount(total, %{id: 3}), do: total * 0.9 def apply_discount(total, %{id: 4}), do: total * 0.9 def apply_discount(total, _), do: total # Rule 4 def send_message_subscription(%{id: 3}, _), do: nil def send_message_subscription(%{id: 4}, _), do: nil def send_message_subscription(subscription, user), do: Subscription.send_email_upgrade(subscription, user) # Rule 5 def print(user, order) do # ... end end defmodule UserNotification do def send_message_subscription(%{id: 3}, _), do: nil def send_message_subscription(%{id: 4}, _), do: nil def send_message_subscription(subscription, user) do Subscription.send_email_upgrade(subscription, user) end end Extract module

Slide 112

Slide 112 text

defmodule ShoppingCart do # Rule 1 def calculate_total(items, subscription) do # ... end # Rule 2 def calculate_shipping(zip_code, %{id: 3}), do: 0.0 def calculate_shipping(zip_code, %{id: 4}), do: 0.0 def calculate_shipping(zip_code, _) do 10.0 * Location.calculate(zip_code) end # Rule 3 def apply_discount(total, %{id: 3}), do: total * 0.9 def apply_discount(total, %{id: 4}), do: total * 0.9 def apply_discount(total, _), do: total # Rule 4 def send_message_subscription(%{id: 3}, _), do: nil def send_message_subscription(%{id: 4}, _), do: nil def send_message_subscription(subscription, user), do: Subscription.send_email_upgrade(subscription, user) # Rule 5 def print(user, order) do # ... end end Before: #5 Rules

Slide 113

Slide 113 text

defmodule ShoppingCart do # Rule 1 def calculate_total(items, subscription) do # ... end # Rule 2 def calculate_shipping(zip_code, %{id: 3}), do: 0.0 def calculate_shipping(zip_code, %{id: 4}), do: 0.0 def calculate_shipping(zip_code, _) do 10.0 * Location.calculate(zip_code) end # Rule 3 def apply_discount(total, %{id: 3}), do: total * 0.9 def apply_discount(total, %{id: 4}), do: total * 0.9 def apply_discount(total, _), do: total # Rule 5 def print(user, order) do # ... end end After: #4 Rules

Slide 114

Slide 114 text

Code Smell: "Long Function"

Slide 115

Slide 115 text

Refactorings Replace Temp with Query, Introduce Parameter Object, Preserve Whole Object, Split Loop, Replace Function with Command, Decompose Conditional, Replace Conditional with Polymorphism Code Smell Long Function

Slide 116

Slide 116 text

#

Slide 117

Slide 117 text

No content

Slide 118

Slide 118 text

About cognition process in programming activities

Slide 119

Slide 119 text

#

Slide 120

Slide 120 text

Small change across several different files Code Smell: "Shotgun Surgery"

Slide 121

Slide 121 text

Refactorings Move Function, Move Field, Combine Functions into Class, Combine Functions into Transform, Split Phase, Inline Function, Inline Class Code Smell Shotgun Surgery

Slide 122

Slide 122 text

defmodule ShoppingCart do def calculate_shipping(_zip_code, %{id: 3}), do: 0.0 def calculate_shipping(_zip_code, %{id: 4}), do: 0.0 def calculate_shipping(zip_code, _), do: 10.0 * Location.calculate(zip_code) def apply_discount(total, %{id: 3}), do: total * 0.95 def apply_discount(total, %{id: 4}), do: total * 0.9 def apply_discount(total, _), do: total end

Slide 123

Slide 123 text

defmodule ShoppingCart do def calculate_shipping(_zip_code, %{id: 3}), do: 0.0 def calculate_shipping(_zip_code, %{id: 4}), do: 0.0 def calculate_shipping(zip_code, _), do: 10.0 * Location.calculate(zip_code) def apply_discount(total, %{id: 3}), do: total * 0.95 def apply_discount(total, %{id: 4}), do: total * 0.9 def apply_discount(total, _), do: total end Enum.member?([3, 4], subscription.id)

Slide 124

Slide 124 text

defmodule ShoppingCart do def calculate_shipping(_zip_code, %{id: 3}), do: 0.0 def calculate_shipping(_zip_code, %{id: 4}), do: 0.0 def calculate_shipping(zip_code, _), do: 10.0 * Location.calculate(zip_code) def apply_discount(total, %{id: 3}), do: total * 0.95 def apply_discount(total, %{id: 4}), do: total * 0.9 def apply_discount(total, _), do: total end Enum.member?([3, 4], subscription.id)

Slide 125

Slide 125 text

defmodule ShoppingCart do def calculate_shipping(_zip_code, %{id: 3}), do: 0.0 def calculate_shipping(_zip_code, %{id: 4}), do: 0.0 def calculate_shipping(zip_code, _), do: 10.0 * Location.calculate(zip_code) def apply_discount(total, %{id: 3}), do: total * 0.95 def apply_discount(total, %{id: 4}), do: total * 0.9 def apply_discount(total, _), do: total end Enum.member?([3, 4], subscription.id)

Slide 126

Slide 126 text

defmodule ShoppingCart do def calculate_shipping(_zip_code, %{id: 3}), do: 0.0 def calculate_shipping(_zip_code, %{id: 4}), do: 0.0 def calculate_shipping(_zip_code, %{id: 8}), do: 0.0 def calculate_shipping(zip_code, _), do: 10.0 * Location.calculate(zip_code) def apply_discount(total, %{id: 3}), do: total * 0.95 def apply_discount(total, %{id: 4}), do: total * 0.9 def apply_discount(total, %{id: 8}), do: total * 0.85 def apply_discount(total, _), do: total end Enum.member?([3, 4, 8], subscription.id)

Slide 127

Slide 127 text

defmodule SubscriptionRenewal do def execute(%{id: id}, user) do case id do 3 -> Billing.perform(user, id, 90.0) 4 -> Billing.perform(user, id, 95.0) 8 -> Billing.perform(user, id, 100.0) end end end Another small change after global search

Slide 128

Slide 128 text

defmodule SubscriptionRenewal do def execute(%{id: id}, user) do case id do 3 -> Billing.perform(user, id, 90.0) 4 -> Billing.perform(user, id, 95.0) 8 -> Billing.perform(user, id, 100.0) end end end Another small change after global search

Slide 129

Slide 129 text

Refactoring: "Move Field"

Slide 130

Slide 130 text

defmodule ShoppingCart do def calculate_shipping(_zip_code, %{id: 3}), do: 0.0 def calculate_shipping(_zip_code, %{id: 4}), do: 0.0 def calculate_shipping(_zip_code, %{id: 8}), do: 0.0 def calculate_shipping(zip_code, _), do: 10.0 * Location.calculate(zip_code) def apply_discount(total, %{id: 3}), do: total * 0.95 def apply_discount(total, %{id: 4}), do: total * 0.9 def apply_discount(total, %{id: 8}), do: total * 0.85 def apply_discount(total, _), do: total end Enum.member?([3, 4, 8], subscription.id) Shipping Discount

Slide 131

Slide 131 text

defmodule SubscriptionRenewal do def execute(%{id: id}, user) do case id do 3 -> Billing.perform(user, id, 90.0) 4 -> Billing.perform(user, id, 95.0) 8 -> Billing.perform(user, id, 100.0) end end end Renewal

Slide 132

Slide 132 text

defmodule Subscription do defstruct [:id, :price, :shipping_tax, :discount_rate] def available_subscriptions do [ %Subscription{id: 3, price: 90.0, shipping_tax: 0.0, discount_rate: 0.95}, %Subscription{id: 4, price: 95.0, shipping_tax: 0.0, discount_rate: 0.9}, %Subscription{id: 8, price: 100.0, shipping_tax: 0.0, discount_rate: 0.85} ] end end

Slide 133

Slide 133 text

defmodule Subscription do defstruct [:id, :price, :shipping_tax, :discount_rate] def available_subscriptions do [ %Subscription{id: 3, price: 90.0, shipping_tax: 0.0, discount_rate: 0.95}, %Subscription{id: 4, price: 95.0, shipping_tax: 0.0, discount_rate: 0.9}, %Subscription{id: 8, price: 100.0, shipping_tax: 0.0, discount_rate: 0.85} ] end end

Slide 134

Slide 134 text

defmodule SubscriptionRenewal do def execute(%{id: id}, user) do case id do 3 -> Billing.perform(user, id, 90.0) 4 -> Billing.perform(user, id, 95.0) 8 -> Billing.perform(user, id, 100.0) end end end

Slide 135

Slide 135 text

defmodule SubscriptionRenewal do # ... def execute(subscription, user) do Billing.perform(user, subscription.id, subscription.price) end end

Slide 136

Slide 136 text

defmodule ShoppingCart do def calculate_shipping(_zip_code, %{id: 3}), do: 0.0 def calculate_shipping(_zip_code, %{id: 4}), do: 0.0 def calculate_shipping(_zip_code, %{id: 8}), do: 0.0 def calculate_shipping(zip_code, _), do: 10.0 * Location.calculate(zip_code) def apply_discount(total, %{id: 3}), do: total * 0.95 def apply_discount(total, %{id: 4}), do: total * 0.9 def apply_discount(total, %{id: 8}), do: total * 0.85 def apply_discount(total, _), do: total end Enum.member?([3, 4, 8], subscription.id)

Slide 137

Slide 137 text

defmodule ShoppingCart do # ... def calculate_shipping(_, %Subscription{shipping_tax: shipping_tax}), do: shipping_tax def calculate_shipping(zip_code, _), do: 10.0 * Location.calculate(zip_code) def apply_discount(total, %Subscription{discount_rate: discount_rate}), do: total * discount_rate def apply_discount(total, _), do: total end

Slide 138

Slide 138 text

#

Slide 139

Slide 139 text

These examples are for illustrative purposes Be mindful about issues when using float for monetary calculations (not only in Elixir!)

Slide 140

Slide 140 text

https://hexdocs.pm/elixir/1.15.5/Float.html

Slide 141

Slide 141 text

https://www.smbc-comics.com/?id=2999 0.1 + 0.2 == 0.3 0.1 + 0.2 == 0.30000000000000004 > false > true https://0.30000000000000004.com/ Unexpected loss of precision during rounding

Slide 142

Slide 142 text

#

Slide 143

Slide 143 text

Not all of the traditional code smells are applicable to Elixir context

Slide 144

Slide 144 text

Loops Lazy Element Speculative Generality Temporary Field Message Chains Middle Man Insider Trading Large Class Alternative Classes with Different Interfaces Data Class Refused Bequest Comments Mysterious Name Duplicated Code Long Function Long Parameter List Global Data Mutable Data Divergent Change Shotgun Surgery Feature Envy Data Clumps Primitive Obsession Repeated Switches 2018

Slide 145

Slide 145 text

https://doi.org/10.1007/s10664-023-10343-6

Slide 146

Slide 146 text

Prof. Marco Tulio Valente Prof. Lucas Francisco da Matta Vegi Federal University of Minas Gerais (UFMG) Belo Horizonte, Minas Gerais, Brazil

Slide 147

Slide 147 text

ELIXIR CODE SMELLS

Slide 148

Slide 148 text

About sub-optimal code structures that can harm the internal quality of Elixir systems

Slide 149

Slide 149 text

14 Design-related smells 09 Low-level concerns smells

Slide 150

Slide 150 text

https://doi.org/10.1007/s10664-023-10343-6

Slide 151

Slide 151 text

https://github.com/lucasvegi/Elixir-Code-Smells

Slide 152

Slide 152 text

#

Slide 153

Slide 153 text

Design-related smells

Slide 154

Slide 154 text

Code Smell: "Unrelated multi-clause function"

Slide 155

Slide 155 text

Multi-clause functions in Elixir Multi-clause functions to group unrelated functionalities Code Smell Unrelated multi-clause function = Code smell != Code smell

Slide 156

Slide 156 text

No content

Slide 157

Slide 157 text

def update(%User{} = user, new_email) when new_email != nil do %{user | email: new_email} end def update(%User{} = user, _new_email), do: user def update(%User.PaymentMethod{} = pay_method, default?) do %{pay_method | default: default?} end def update(%User.Notification{} = notification, channel) do %{notification | channel: channel} end

Slide 158

Slide 158 text

def update(%User{} = user, new_email) when new_email != nil do %{user | email: new_email} end def update(%User{} = user, _new_email), do: user def update(%User.PaymentMethod{} = pay_method, default?) do %{pay_method | default: default?} end def update(%User.Notification{} = notification, channel) do %{notification | channel: channel} end

Slide 159

Slide 159 text

def update(%User{} = user, new_email) when new_email != nil do %{user | email: new_email} end def update(%User{} = user, _new_email), do: user def update(%User.PaymentMethod{} = pay_method, default?) do %{pay_method | default: default?} end def update(%User.Notification{} = notification, channel) do %{notification | channel: channel} end #3 #2 #1

Slide 160

Slide 160 text

Refactoring

Slide 161

Slide 161 text

def update(%User{} = user, new_email) when new_email != nil do %{user | email: new_email} end def update(%User{} = user, _new_email), do: user def update(%User.PaymentMethod{} = pay_method, default?) do %{pay_method | default: default?} end def update(%User.Notification{} = notification, channel) do %{notification | channel: channel} end #1

Slide 162

Slide 162 text

@doc """ Updates a user's email address. """ def update_user_email(%User{} = user, new_email) when new_email != nil do %{user | email: new_email} end def update_user_email(%User{} = user, _new_email), do: user #1 New function

Slide 163

Slide 163 text

def update(%User{} = user, new_email) when new_email != nil do %{user | email: new_email} end def update(%User{} = user, _new_email), do: user def update(%User.PaymentMethod{} = pay_method, default?) do %{pay_method | default: default?} end def update(%User.Notification{} = notification, channel) do %{notification | channel: channel} end #2

Slide 164

Slide 164 text

@doc """ Updates user's default payment method. """ def update_user_payment_method(%User.PaymentMethod{} = pay_method, default?) do %{pay_method | default: default?} end #2 New function

Slide 165

Slide 165 text

def update(%User{} = user, new_email) when new_email != nil do %{user | email: new_email} end def update(%User{} = user, _new_email), do: user def update(%User.PaymentMethod{} = pay_method, default?) do %{pay_method | default: default?} end def update(%User.Notification{} = notification, channel) do %{notification | channel: channel} end #3

Slide 166

Slide 166 text

@doc """ Updates a user's notification preferences. """ def update_user_notification(%User.Notification{} = notification, channel) do %{notification | channel: channel} end #3 New function

Slide 167

Slide 167 text

Code Smell: "Compile-time global configuration"

Slide 168

Slide 168 text

Code Smell Compile-time global configuration Module attributes Defined at compile-time Application Environment May not be available in memory during compile-time/build-time *warnings or errors can be triggered by Elixir

Slide 169

Slide 169 text

defmodule DashSplitter do @parts Application.fetch_env!(:app_config, :parts) def split(string) when is_binary(string) do String.split(string, "-", parts: @parts) end end

Slide 170

Slide 170 text

defmodule DashSplitter do @parts Application.fetch_env!(:app_config, :parts) def split(string) when is_binary(string) do String.split(string, "-", parts: @parts) end end ** (ArgumentError) could not fetch application environment :parts for application :app_config because the application was not loaded nor configured

Slide 171

Slide 171 text

defmodule DashSplitter do @parts Application.compile_env(:app_config, :parts, 3) def split(string) when is_binary(string) do String.split(string, "-", parts: @parts) end end *with default

Slide 172

Slide 172 text

Compile time vs Runtime https://elixir-lang.org/getting-started/mix-otp/config-and-releases.html#configuration

Slide 173

Slide 173 text

#

Slide 174

Slide 174 text

Low-level concerns smells

Slide 175

Slide 175 text

Code Smell: "Complex else clauses in with"

Slide 176

Slide 176 text

`with` statement with a single complex `else` block Code Smell Complex else clauses in with = Code smell `with` statement + else != Code smell

Slide 177

Slide 177 text

def open_decoded_file(path) do with {:ok, encoded} <- File.read(path), {:ok, value} <- Base.decode64(encoded) do value else {:error, _} -> :badfile :error -> :badencoding end end Code Smell Complex else clauses in with

Slide 178

Slide 178 text

def open_decoded_file(path) do with {:ok, encoded} <- File.read(path), {:ok, value} <- Base.decode64(encoded) do value else {:error, _} -> :badfile :error -> :badencoding end end Code Smell Complex else clauses in with Difficult to know from which clause the error value came.

Slide 179

Slide 179 text

Refactoring

Slide 180

Slide 180 text

def open_decoded_file(path) do with {:ok, encoded} <- File.read(path), {:ok, value} <- Base.decode64(encoded) do value else {:error, _} -> :badfile :error -> :badencoding end end Code Smell Complex else clauses in with

Slide 181

Slide 181 text

Code Smell Complex else clauses in with def open_decoded_file(path) do with {:ok, encoded} <- file_read(path), {:ok, value} <- base_decode64(encoded) do value end end

Slide 182

Slide 182 text

Code Smell Complex else clauses in with def open_decoded_file(path) do with {:ok, encoded} <- file_read(path), {:ok, value} <- base_decode64(encoded) do value end end

Slide 183

Slide 183 text

defp file_read(path) do case File.read(path) do {:ok, contents} -> {:ok, contents} {:error, _} -> :badfile end end Code Smell Complex else clauses in with

Slide 184

Slide 184 text

defp file_read(path) do case File.read(path) do {:ok, contents} -> {:ok, contents} {:error, _} -> :badfile end end Code Smell Complex else clauses in with

Slide 185

Slide 185 text

Code Smell Complex else clauses in with def open_decoded_file(path) do with {:ok, encoded} <- file_read(path), {:ok, value} <- base_decode64(encoded) do value end end

Slide 186

Slide 186 text

Code Smell Complex else clauses in with defp base_decode64(contents) do case Base.decode64(contents) do {:ok, contents} -> {:ok, contents} :error -> :badencoding end end

Slide 187

Slide 187 text

Code Smell Complex else clauses in with defp base_decode64(contents) do case Base.decode64(contents) do {:ok, contents} -> {:ok, contents} :error -> :badencoding end end

Slide 188

Slide 188 text

Code Smell: "Dynamic Atom Creation"

Slide 189

Slide 189 text

Creating atoms dynamically with no control Code Smell Complex else clauses in with = Code smell Creating atoms dynamically != Code smell *Atoms are constants that are not garbage collected by the Erlang VM

Slide 190

Slide 190 text

defmodule Identifier do def generate(id) when is_bitstring(id) do String.to_atom(id) #<= dynamic atom creation!! end end

Slide 191

Slide 191 text

defmodule Identifier do def generate(id) when is_bitstring(id) do String.to_atom(id) #<= dynamic atom creation!! end end

Slide 192

Slide 192 text

Refactoring

Slide 193

Slide 193 text

defmodule Identifier do def generate(id) when is_bitstring(id) do String.to_atom(id) #<= dynamic atom creation!! end end

Slide 194

Slide 194 text

defmodule Identifier do def generate(id) when is_bitstring(id) do String.to_existing_atom(id) #<= maps a string to an existing atom! end end

Slide 195

Slide 195 text

Elixir 1.16

Slide 196

Slide 196 text

https://hexdocs.pm/elixir/main/what-anti-patterns.html

Slide 197

Slide 197 text

Code-related anti-patterns Design-related anti-patterns Process-related anti-patterns Meta-programming anti-patterns

Slide 198

Slide 198 text

Anti-pattern structure Name Unique, To facilitate communication Problem How it can harm code quality What impacts it can have for developers Example Code + textual description Refactoring Suggested changes to improve code quality

Slide 199

Slide 199 text

Anti-pattern structure Name Unique, To facilitate communication Problem How it can harm code quality What impacts it can have for developers Example Code + textual description Refactoring Suggested changes to improve code quality

Slide 200

Slide 200 text

No content

Slide 201

Slide 201 text

WHAT'S NEXT?

Slide 202

Slide 202 text

#1 - Catalog of Refactorings for Elixir Code Smells

Slide 203

Slide 203 text

https://homepages.dcc.ufmg.br/~mtov/pub/2023-icsme-nier-elixir.pdf 2023

Slide 204

Slide 204 text

#2 - Tooling

Slide 205

Slide 205 text

hexdocs.pm/credo Improving

Slide 206

Slide 206 text

A.I.? GH Copilot ChatGPT etc.

Slide 207

Slide 207 text

#3 - Metrics?

Slide 208

Slide 208 text

Cyclomatic complexity

Slide 209

Slide 209 text

No content

Slide 210

Slide 210 text

ABC Software Metric

Slide 211

Slide 211 text

No content

Slide 212

Slide 212 text

WRAPPING UP

Slide 213

Slide 213 text

History Traditional and Elixir code smells Impact of code smells over time Refactoring techniques

Slide 214

Slide 214 text

# Understability and changeability matter

Slide 215

Slide 215 text

Research indicates that almost 60% of programmers’ time is spent understanding code, rather than writing code. From The Programmer's Brain - Felienne Hermans

Slide 216

Slide 216 text

# Automated tests + Refactoring =

Slide 217

Slide 217 text

https://martinfowler.com/bliki/TestCoverage.html

Slide 218

Slide 218 text

https://martinfowler.com/articles/practical-test-pyramid.html

Slide 219

Slide 219 text

# Refactoring != Revolution

Slide 220

Slide 220 text

It's all about accumulative improvements

Slide 221

Slide 221 text

#

Slide 222

Slide 222 text

Legacy code in Elixir

Slide 223

Slide 223 text

How is your Elixir codebase?

Slide 224

Slide 224 text

Is it EASY TO CHANGE?

Slide 225

Slide 225 text

speakerdeck.com/elainenaomi elainenaomi.dev Muito obrigada illustrations from undraw.co translation: thank you so much!

Slide 226

Slide 226 text

railsgirls.com.br

Slide 227

Slide 227 text

2018 2020 2021 2019 2020 2018

Slide 228

Slide 228 text

2022

Slide 229

Slide 229 text

No content

Slide 230

Slide 230 text

CARNIVAL EDITION ELIXIR

Slide 231

Slide 231 text

No content