Code Smells & Refactoring

1.

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

2.

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

3.

None

4.

None

5.

None

6.

None

7.

None

8.

Know your tools

9.

DRY don’t repeat yourself

10.

SLAP Single Level of Abstraction Principle

11.

Law of Demeter

12.

Out side of the box

13.

Code smells

14.

None

15.

None

16.

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

17.

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

18.

None

19.

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

20.

None

21.

None

22.

None

23.

Exam ExamQuestion ExamEntry ExamAnswer

24.

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

25.

Long Method

26.

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

27.

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

28.


29.


30.


31.


32.


33.


34.


35.


36.

Text

37.

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

38.


39.


40.

Conditional complexity

41.

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

42.

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

43.

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

44.

None

45.

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

46.

None

47.

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

48.

Text

49.

= 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

50.


51.


52.

= 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

53.

Text

54.


55.


56.


57.

Code duplication

58.

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

59.


60.


61.


62.

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

63.

Text

64.


65.


66.

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

67.

Text

68.


69.


70.


71.

Feature Envy

72.

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

73.


74.

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

75.

Text

76.

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

77.

Text

78.


79.


80.


81.

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

82.

Text

83.


84.


85.

Message chain

86.

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

87.

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

88.

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

89.

Text

90.

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

Code Smells & Refactoring

Code Smells & Refactoring

Want more?

Transcript