Code Smells & Refactoring

Slide 1

Slide 1 text

Radoslav Stankov VarnaConf 2013 20/07/2013 Code Smells & Refactoring

Slide 2

Slide 2 text

Кой съм аз? @rstankov http://rstankov.com http://github.com/rstankov

Slide 3

Slide 3 text

No content

Slide 4

Slide 4 text

No content

Slide 5

Slide 5 text

No content

Slide 6

Slide 6 text

No content

Slide 7

Slide 7 text

No content

Slide 8

Slide 8 text

Know your tools

Slide 9

Slide 9 text

DRY don’t repeat yourself

Slide 10

Slide 10 text

SLAP Single Level of Abstraction Principle

Slide 11

Slide 11 text

Law of Demeter

Slide 12

Slide 12 text

Out side of the box

Slide 13

Slide 13 text

Code smells

Slide 14

Slide 14 text

No content

Slide 15

Slide 15 text

No content

Slide 16

Slide 16 text

Code smell is any symptom in the source code of a program that possibly indicates a deeper problem. Code smells

Slide 17

Slide 17 text

Code smells are usually not bugs—they are not technically incorrect and don't currently prevent the program from functioning. Instead, they indicate weaknesses in design that may be slowing down development or increasing the risk of bugs or failures in the future. Code smells

Slide 18

Slide 18 text

No content

Slide 19

Slide 19 text

Code refactoring is a disciplined technique for restructuring an existing body of code, altering its internal structure without changing its external behavior Refactoring

Slide 20

Slide 20 text

No content

Slide 21

Slide 21 text

No content

Slide 22

Slide 22 text

No content

Slide 23

Slide 23 text

Exam ExamQuestion ExamEntry ExamAnswer

Slide 24

Slide 24 text

class ExamEntry < ActiveRecord::Base def result correct_answers = [] for answer in answers question = answer.question if (question.type == 'single' && question.correct_answer == answer.text) || (question.type == 'multi' && question.correct_answers.include?(answer.text)) correct_answers << answer end end wrong_answers = [] for answer in answers question = answer.question unless (question.type == 'single' && question.correct_answer == answer.text) || (question.type == 'multi' && question.correct_answers.include?(answer.text)) wrong_answers << answer end end score = 0 for answer in correct_answers score += answer.question.points end { correct_answers: correct_answers, wrong_answers: wrong_answers, score: score } end end

Slide 25

Slide 25 text

Long Method

Slide 26

Slide 26 text

A method, function, or procedure that has grown too large. Long Method

Slide 27

Slide 27 text

describe ExamEntry do describe "#result" do it "works ( I hope :P )" do exam = create :exam single_1 = create :single_question, exam: exam, points: 1 single_2 = create :single_question, exam: exam, points: 2 multi_1 = create :multi_question, exam: exam, points: 1 multi_2 = create :multi_question, exam: exam, points: 2 entry = create(:exam_entry, answers: [ create(:answer, question: single_1, text: single_1.correct_answer), create(:answer, question: single_2, text: '-wrong-answer-'), create(:answer, question: multi_1, text: multi_1.correct_answers.first), create(:answer, question: multi_2, text: '-wrong-answer-') ]) result = entry.result result.keys.should eq [:correct_answers, :wrong_answers, :score] result[:correct_answers].map(&:question).should eq [single_1, multi_1] result[:wrong_answers].map(&:question).should eq [single_2, multi_2] result[:score].should eq single_1.points + multi_1.points end end end

Slide 28

Slide 28 text

Slide 29

Slide 29 text

Slide 30

Slide 30 text

Slide 31

Slide 31 text

Slide 32

Slide 32 text

Slide 33

Slide 33 text

Slide 34

Slide 34 text

Slide 35

Slide 35 text

Slide 36

Slide 36 text

Text

Slide 37

Slide 37 text

def result correct_answers = [] for answer in answers question = answer.question if (question.type == 'single' && question.correct_answer == answer.text) || (question.type == 'multi' && question.correct_answers.include?(answer.text)) correct_answers << answer end end wrong_answers = [] for answer in answers question = answer.question unless (question.type == 'single' && question.correct_answer == answer.text) || (question.type == 'multi' && question.correct_answers.include?(answer.text)) wrong_answers << answer end end score = 0 for answer in correct_answers score += answer.question.points end { correct_answers: correct_answers, wrong_answers: wrong_answers, score: score } end

Slide 38

Slide 38 text

Slide 39

Slide 39 text

Slide 40

Slide 40 text

Conditional complexity

Slide 41

Slide 41 text

Branches that check lots of unrelated conditions and edge cases that don't seem to capture the meaning of a block of code. Complex conditionals

Slide 42

Slide 42 text

(question.type == 'single' && question.correct_answer == answer.text) || (question.type == 'multi' && question.correct_answers.include?(answer.text))

Slide 43

Slide 43 text

describe ExamQuestion do describe "#correct_answer?" do context "single" do it "returns true if answer is correct" it "returns false if answer is wrong" end context "multi" do it "returns true if answer included in answers list" it "returns false if answer isn’t included in answers list" end end end

Slide 44

Slide 44 text

No content

Slide 45

Slide 45 text

class ExamQuestion < ActiveRecord::Base def correct_answer?(answer) if type == 'single' correct_answer == answer else correct_answers.include? answer end end end

Slide 46

Slide 46 text

No content

Slide 47

Slide 47 text

class ExamQuestion < ActiveRecord::Base def correct_answer?(answer) if type == 'single' correct_answer == answer else correct_answers.include? answer end end end

Slide 48

Slide 48 text

Text

Slide 49

Slide 49 text

def result correct_answers = [] for answer in answers question = answer.question if (question.type == 'single' && question.correct_answer == answer.text) || (question.t correct_answers << answer end end wrong_answers = [] for answer in answers question = answer.question unless (question.type == 'single' && question.correct_answer == answer.text) || (questi wrong_answers << answer end end score = 0 for answer in correct_answers score += answer.question.points end { correct_answers: correct_answers, wrong_answers: wrong_answers, score: score } end

Slide 50

Slide 50 text

Slide 51

Slide 51 text

Slide 52

Slide 52 text

def result correct_answers = [] for answer in answers question = answer.question if question.correct_answer? answer.text correct_answers << answer end end wrong_answers = [] for answer in answers question = answer.question unless question.correct_answer? answer.text wrong_answers << answer end end score = 0 for answer in correct_answers score += answer.question.points end { correct_answers: correct_answers, wrong_answers: wrong_answers, score: score } end

Slide 53

Slide 53 text

Text

Slide 54

Slide 54 text

Slide 55

Slide 55 text

Slide 56

Slide 56 text

Slide 57

Slide 57 text

Code duplication

Slide 58

Slide 58 text

Identical or very similar code exists in more than one location. Duplicated code

Slide 59

Slide 59 text

Slide 60

Slide 60 text

Slide 61

Slide 61 text

Slide 62

Slide 62 text

def result correct_answers = [] wrong_answers = [] for answer in answers question = answer.question if question.correct_answer? answer.text correct_answers << answer else wrong_answers << answer end end score = 0 for answer in correct_answers score += answer.question.points end { correct_answers: correct_answers, wrong_answers: wrong_answers, score: score } end

Slide 63

Slide 63 text

Text

Slide 64

Slide 64 text

Slide 65

Slide 65 text

Slide 66

Slide 66 text

def result correct_answers = [] wrong_answers = [] score = 0 for answer in answers question = answer.question if question.correct_answer? answer.text correct_answers << answer score += question.points else wrong_answers << answer end end { correct_answers: correct_answers, wrong_answers: wrong_answers, score: score } end

Slide 67

Slide 67 text

Text

Slide 68

Slide 68 text

Slide 69

Slide 69 text

Slide 70

Slide 70 text

Slide 71

Slide 71 text

Feature Envy

Slide 72

Slide 72 text

A class that uses methods of another class excessively. Feature envy

Slide 73

Slide 73 text

Slide 74

Slide 74 text

describe ExamAnswer do describe "#correct?" do it "returns true if answer is correct" it "returns false if answer is wrong" end end

Slide 75

Slide 75 text

Text

Slide 76

Slide 76 text

class ExamAnswer < ActiveRecord::Base def correct? question.correct_answer? text end end

Slide 77

Slide 77 text

Text

Slide 78

Slide 78 text

Slide 79

Slide 79 text

Slide 80

Slide 80 text

Slide 81

Slide 81 text

def result correct_answers = [] wrong_answers = [] score = 0 for answer in answers if answer.correct? correct_answers << answer score += answer.question.points else wrong_answers << answer end end { correct_answers: correct_answers, wrong_answers: wrong_answers, score: score } end

Slide 82

Slide 82 text

Text

Slide 83

Slide 83 text

Slide 84

Slide 84 text

Slide 85

Slide 85 text

Message chain

Slide 86

Slide 86 text

Long sequences of method calls to get routine data. Intermediaries are dependencies in disguise. Message chain

Slide 87

Slide 87 text

describe ExamAnswer do describe "#points" do context "correct answer" do it "returns its question points" end context "wrong answer" do it "returns 0" end end end

Slide 88

Slide 88 text

describe ExamAnswer do describe "#points" do context "correct answer" do it "returns its question points" end context "wrong answer" do it "returns 0" end end end

Slide 89

Slide 89 text

Text

Slide 90

Slide 90 text

class ExamAnswer < ActiveRecord::Base def points return 0 unless correct? question.points end end