Upgrade to Pro — share decks privately, control downloads, hide ads and more …

Why Our Code Smells

Why Our Code Smells

Odors are communication devices. They exist for a reason and are usually trying to tell us something.

Our code smells and it is trying to tell us what is wrong. Does a test case require an abundance of setup? Maybe the code being tested is doing too much, or it is not isolated enough for the test? Does an object have an abundance of instance variables? Maybe it should be split into multiple objects? Is a view brittle? Maybe it is too tightly coupled to a model, or maybe the logic needs abstracted into an object that can be tested?

In this talk, I will walk through code from projects that I work on every day, looking for smells that indicate problems, understanding why it smells, what the smell is trying to tell us, and how to refactor it.

Brandon Keepers

May 23, 2012
Tweet

More Decks by Brandon Keepers

Other Decks in Programming

Transcript

  1. SMELLS
    http://www.flickr.com/photos/kanaka/3480201136/
    WHY OUR CODE

    View full-size slide

  2. github.com/bkeepers
    @bkeepers

    View full-size slide

  3. I AM TIRED OF
    WRITING
    BAD CODE.

    View full-size slide

  4. I AM TIRED OF
    MAINTAINING
    BAD CODE.

    View full-size slide

  5. Kent Beck
    Smalltalk Best Practice Patterns
    “Code doesn’t lie. If
    you’re not listening,
    you won’t hear the
    truths it tells.”

    View full-size slide

  6. A CODE SMELL
    USUALLY INDICATES
    A DEEPER PROBLEM
    IN THE SYSTEM.

    View full-size slide

  7. CODE SMELLS ARE
    HEURISTICS TO
    SUGGEST WHEN TO
    REFACTOR AND WHAT
    TECHNIQUES TO USE.

    View full-size slide

  8. DIVERGENT CHANGE

    View full-size slide

  9. DIVERGENT CHANGE
    Occurs when one class is commonly
    changed in different ways for different
    reasons. Any change to handle a
    variation should change a single class.

    View full-size slide

  10. DIVERGENT CHANGE
    Occurs when one class is commonly
    changed in different ways for different
    reasons. Any change to handle a
    variation should change a single class.
    Refactoring:
    Identify everything that changes for a
    particular cause and use Extract Class
    to put them all together.

    View full-size slide

  11. OUR CODE SMELLS IN
    THE 21 WAYS THAT
    BECK AND FOWLER
    DESCRIBE, BUT…

    View full-size slide

  12. our code smells when
    UNIT TESTS ARE
    COMPLEX & SLOW.
    IT IS TIGHTLY COUPLED
    TO A FRAMEWORK.

    View full-size slide

  13. our code smells when
    UNIT TESTS
    ARE COMPLEX.

    View full-size slide

  14. TDD IS A
    DESIGN
    PROCESS.

    View full-size slide

  15. WHY DO WE WRITE TESTS?

    View full-size slide

  16. WHY DO WE WRITE TESTS?
    1.Guard against regressions

    View full-size slide

  17. WHY DO WE WRITE TESTS?
    1.Guard against regressions
    2.Gain confidence to change

    View full-size slide

  18. WHY DO WE WRITE TESTS?
    1.Guard against regressions
    2.Gain confidence to change
    3.Discover better designs

    View full-size slide

  19. WARNING
    Excessive use of
    quotations ahead.

    View full-size slide

  20. Steve Freeman, Nat Pryce
    Growing Object-Oriented Software, Guided by Tests
    “We find that the effort of writing
    a test first also gives us rapid
    feedback about the quality of our
    design ideas—that making code
    accessible for testing often
    drives it towards being cleaner
    and more modular.”

    View full-size slide

  21. unit tests can be complex when
    OBJECTS ARE
    TOO TIGHTLY
    COUPLED.

    View full-size slide

  22. Exhibit A: GitHub Notifications
    context Notifications::Emails::Message do
    test "#to uses global email" do
    @settings.email :global, '[email protected]'
    assert_equal '[email protected]', @message.to
    end
    test "#body includes comment body" do
    assert_match @comment.body, @message.body
    end
    end

    View full-size slide

  23. Exhibit A: GitHub Notifications
    context Notifications::Emails::Message do
    setup do
    @comment = Issue.make
    @summary = Notifications::Summary.from(@comment)
    @handlers = [Notifications::EmailHandler.new]
    @delivery = Notifications::Delivery.new(
    @summary, @comment, @handlers)
    @settings = Notifications::Settings.new(-1)
    @message = Notifications::Emails::Message.new(
    @delivery, @settings)
    end
    test "#to uses global email" do
    @settings.email :global, '[email protected]'
    assert_equal '[email protected]', @message.to
    end

    View full-size slide

  24. Steve Freeman, Nat Pryce
    Growing Object-Oriented Software, Guided by Tests
    “When we find a feature
    that’s difficult to test, we
    don’t just ask ourselves
    how to test it, but also
    why is it difficult to test.”

    View full-size slide

  25. Exhibit A: GitHub Notifications
    context Notifications::Emails::Message do
    setup do
    @comment = Issue.make
    @summary = Notifications::Summary.from(@comment)
    @handlers = [Notifications::EmailHandler.new]
    @delivery = Notifications::Delivery.new(
    @summary, @comment, @handlers)
    @settings = Notifications::Settings.new(-1)
    @message = Notifications::Emails::Message.new(
    @delivery, @settings)
    end
    test "#to uses global email" do
    @settings.email :global, '[email protected]'
    assert_equal '[email protected]', @message.to
    end
    end

    View full-size slide

  26. Exhibit A: GitHub Notifications
    context Notifications::Emails::Message do
    setup do
    @comment = Issue.make
    @settings = Notifications::Settings.new(-1)
    @message = Notifications::Emails::Message.new(
    @comment, @settings)
    end
    test "#to uses global email" do
    @settings.email :global, '[email protected]'
    assert_equal '[email protected]', @message.to
    end
    end

    View full-size slide

  27. Exhibit A: GitHub Notifications
    context Notifications::Emails::Message do
    setup do
    @comment = Issue.make
    @settings = Notifications::Settings.new(-1)
    @message = Notifications::Emails::Message.new(
    @comment, @settings)
    end
    test "#to uses global email" do
    @settings.email :global, '[email protected]'
    assert_equal '[email protected]', @message.to
    end
    test "#body includes comment body" do
    assert_match @comment.body, @message.body
    end
    end

    View full-size slide

  28. unit tests can be complex when
    OBJECTS ARE
    DOING TOO
    MUCH.

    View full-size slide

  29. Steve Freeman, Nat Pryce
    Growing Object-Oriented Software, Guided by Tests
    “An element’s cohesion is a measure
    of whether its responsibilities form a
    meaningful unit…Think of a machine
    that washes both clothes and dishes
    —it’s unlikely to do both well.”

    View full-size slide

  30. Every class should have a single responsibility,
    and that responsibility should be entirely
    encapsulated by the class.
    SINGLE
    RESPONSIBILITY
    PRINCIPLE

    View full-size slide

  31. Steve Freeman, Nat Pryce
    Growing Object-Oriented Software, Guided by Tests
    “Our heuristic is that we
    should be able to describe
    what an object does
    without using any
    conjunctions (‘and,’ ‘or’).”

    View full-size slide

  32. jQuery(function($) {
    $('#new-status').on('submit', function() {
    $.ajax({
    url: '/statuses',
    type: 'POST',
    dataType: 'json',
    data: {text: $(this).find('textarea').val()},
    success: function(data) {
    $('#statuses').append('' + data.text + '');
    }
    });
    return false;
    });
    });
    example lovingly stolen from @searls

    View full-size slide

  33. The test tells me the code is complex.
    describe("Updating my status", function() {
    var $form, $statuses;
    beforeEach(function(){
    $form = affix('form#new-status');
    $form.affix('textarea').val('sniffing code');
    $statuses = affix('#statuses');
    spyOn($, "ajax");
    $form.trigger('submit');
    });
    it("posts status to the server", function() {
    expect($.ajax).toHaveBeenCalledWith({
    url: '/statuses',
    data: {text: 'sniffing code'},
    success: jasmine.any(Function)
    });
    });

    View full-size slide

  34. The test tells me the code is complex.
    });
    it("posts status to the server", function() {
    expect($.ajax).toHaveBeenCalledWith({
    url: '/statuses',
    data: {text: 'sniffing code'},
    success: jasmine.any(Function)
    });
    });
    describe("with a successful response", function() {
    beforeEach(function() {
    $.ajax.mostRecentCall.args[0].success({
    text: "This is starting stink!"
    });
    });
    it("appends text", function() {
    expect($statuses).toHaveHtml(
    'This is starting stink!');
    });
    });
    });

    View full-size slide

  35. Why is this hard to test?
    jQuery(function($) {
    $('#new-status').on('submit', function() {
    $.ajax({
    url: '/statuses',
    type: 'POST',
    dataType: 'json',
    data: {text: $(this).find('textarea').val()},
    success: function(data) {
    $('#statuses').append('' + data.text + '');
    }
    });
    return false;
    });
    });
    example lovingly stolen from @searls

    View full-size slide

  36. Why is this hard to test?
    jQuery(function($) {
    $('#new-status').on('submit', function() {
    $.ajax({
    url: '/statuses',
    type: 'POST',
    dataType: 'json',
    data: {text: $(this).find('textarea').val()},
    success: function(data) {
    $('#statuses').append('' + data.text + '');
    }
    });
    return false;
    });
    });
    1. page event
    example lovingly stolen from @searls

    View full-size slide

  37. Why is this hard to test?
    jQuery(function($) {
    $('#new-status').on('submit', function() {
    $.ajax({
    url: '/statuses',
    type: 'POST',
    dataType: 'json',
    data: {text: $(this).find('textarea').val()},
    success: function(data) {
    $('#statuses').append('' + data.text + '');
    }
    });
    return false;
    });
    });
    1. page event
    2. user event
    example lovingly stolen from @searls

    View full-size slide

  38. Why is this hard to test?
    jQuery(function($) {
    $('#new-status').on('submit', function() {
    $.ajax({
    url: '/statuses',
    type: 'POST',
    dataType: 'json',
    data: {text: $(this).find('textarea').val()},
    success: function(data) {
    $('#statuses').append('' + data.text + '');
    }
    });
    return false;
    });
    });
    1. page event
    2. user event
    3. network IO
    example lovingly stolen from @searls

    View full-size slide

  39. Why is this hard to test?
    jQuery(function($) {
    $('#new-status').on('submit', function() {
    $.ajax({
    url: '/statuses',
    type: 'POST',
    dataType: 'json',
    data: {text: $(this).find('textarea').val()},
    success: function(data) {
    $('#statuses').append('' + data.text + '');
    }
    });
    return false;
    });
    });
    1. page event
    2. user event
    3. network IO
    4. user input
    example lovingly stolen from @searls

    View full-size slide

  40. Why is this hard to test?
    jQuery(function($) {
    $('#new-status').on('submit', function() {
    $.ajax({
    url: '/statuses',
    type: 'POST',
    dataType: 'json',
    data: {text: $(this).find('textarea').val()},
    success: function(data) {
    $('#statuses').append('' + data.text + '');
    }
    });
    return false;
    });
    });
    1. page event
    2. user event
    3. network IO
    4. user input
    5. network event
    example lovingly stolen from @searls

    View full-size slide

  41. Why is this hard to test?
    jQuery(function($) {
    $('#new-status').on('submit', function() {
    $.ajax({
    url: '/statuses',
    type: 'POST',
    dataType: 'json',
    data: {text: $(this).find('textarea').val()},
    success: function(data) {
    $('#statuses').append('' + data.text + '');
    }
    });
    return false;
    });
    });
    1. page event
    2. user event
    3. network IO
    4. user input
    5. network event
    6. HTML templating
    example lovingly stolen from @searls

    View full-size slide

  42. jQuery(function($) {
    $('#new-status').on('submit', function() {
    $.ajax({
    url: '/statuses',
    type: 'POST',
    dataType: 'json',
    data: {text: $(this).find('textarea').val()},
    success: function(data) {
    $('#statuses').append('' + data.text + '');
    }
    });
    return false;
    });
    });
    So we start to refactor…

    View full-size slide

  43. jQuery(function($) {
    $('#new-status').on('submit', function() {
    $.ajax({
    url: '/statuses',
    type: 'POST',
    dataType: 'json',
    data: {text: $(this).find('textarea').val()},
    success: function(data) {
    $('#statuses').append('' + data.text + '');
    }
    });
    return false;
    });
    });
    Refactor to use a model

    View full-size slide

  44. Refactor to use a model
    jQuery(function($) {
    var statuses = new Collection.Statuses();
    $('#new-status').on('submit', function() {
    statuses.create({text: $(this).find('textarea').val()});
    return false;
    });
    statuses.on('add', function(status) {
    $('#statuses').append(
    '' + status.get('text') + '');
    });
    });

    View full-size slide

  45. Refactor to use a model
    jQuery(function($) {
    var statuses = new Collection.Statuses();
    $('#new-status').on('submit', function() {
    statuses.create({text: $(this).find('textarea').val()});
    return false;
    });
    statuses.on('add', function(status) {
    $('#statuses').append(
    '' + status.get('text') + '');
    });
    });
    RESPONSIBILITY:
    Sync state with server

    View full-size slide

  46. Refactor handling of user input
    jQuery(function($) {
    var statuses = new Collection.Statuses();
    $('#new-status').on('submit', function() {
    statuses.create({text: $(this).find('textarea').val()});
    return false;
    });
    statuses.on('add', function(status) {
    $('#statuses').append(
    '' + status.get('text') + '');
    });
    });

    View full-size slide

  47. Refactor handling of user input
    jQuery(function($) {
    var statuses = new Collection.Statuses();
    new View.PostStatus({collection: statuses});
    statuses.on('add', function(status) {
    $('#statuses').append(
    '' + status.get('text') + '');
    });
    });

    View full-size slide

  48. Refactor handling of user input
    jQuery(function($) {
    var statuses = new Collection.Statuses();
    new View.PostStatus({collection: statuses});
    statuses.on('add', function(status) {
    $('#statuses').append(
    '' + status.get('text') + '');
    });
    });
    RESPONSIBILITY:
    Create statuses from user input

    View full-size slide

  49. jQuery(function($) {
    var statuses = new Collection.Statuses();
    new View.PostStatus({collection: statuses});
    statuses.on('add', function(status) {
    $('#statuses').append(
    '' + status.get('text') + '');
    });
    });
    Refactor templating

    View full-size slide

  50. Refactor templating
    jQuery(function($) {
    var statuses = new Collection.Statuses();
    new View.PostStatus({collection: statuses});
    new View.StatusList({collection: statuses});
    });

    View full-size slide

  51. Refactor templating
    jQuery(function($) {
    var statuses = new Collection.Statuses();
    new View.PostStatus({collection: statuses});
    new View.StatusList({collection: statuses});
    });
    RESPONSIBILITY:
    Render statuses to the page

    View full-size slide

  52. jQuery(function($) {
    var statuses = new Collection.Statuses();
    new View.PostStatus({collection: statuses});
    new View.StatusList({collection: statuses});
    });
    RESPONSIBILITY:
    Initialize application on page load

    View full-size slide

  53. Our tests only have one concern
    describe("View.StatusList", function() {
    beforeEach(function() {
    $el = $('');
    collection = new Backbone.Collection();
    view = new View.StatusList({
    el: $el,
    collection: collection
    });
    });
    it("appends newly added items", function() {
    collection.add({text: 'this is fun!'});
    expect($el.find('li').length).toBe(1);
    expect($el.find('li').text()).toEqual('this is fun!');
    });
    });

    View full-size slide

  54. PAY ATTENTION TO
    TESTING PAINS AND
    ADJUST THE DESIGN
    ACCORDINGLY.

    View full-size slide

  55. Steve Freeman, Nat Pryce
    Growing Object-Oriented Software, Guided by Tests
    Poor quality tests can slow
    development to a crawl, and
    poor internal quality of the
    system being tested will result
    in poor quality tests.

    View full-size slide

  56. Christian Johansen
    Test-Driven JavaScript Development
    “If you write bad unit tests, you
    might find that you gain none of
    the benefits, and instead are
    stuck with a bunch of tests that
    are time-consuming and hard to
    maintain.”

    View full-size slide

  57. our code smells when
    UNIT TESTS
    ARE SLOW.

    View full-size slide

  58. $ bx rake test:units
    ...............................................................................................
    ...............................................................................................
    ...............................................................................................
    ...............................................................................................
    ...............................................................................................
    ...............................................................................................
    ...............................................................................................
    ...............................................................................................
    ...............................................................................................
    ...............................................................................................
    ...............................................................................................
    ...............................................................................................
    ...............................................................................................
    ...............................................................................................
    ...............................................................................................
    ...............................................................................................
    ...............................................................................................
    ...............................................................................................
    ...............................................................................................
    ...............................................................................................
    ...............................................................................................
    ...............................................................................................
    ...............................................................................................
    ........................................................................................
    Finished in 274.623286 seconds.
    2273 tests, 6765 assertions, 0 failures, 0 errors

    View full-size slide

  59. WHAT’S WRONG WITH SLOW TESTS?

    View full-size slide

  60. You don't run them often
    WHAT’S WRONG WITH SLOW TESTS?

    View full-size slide

  61. You don't run them often
    You waste time waiting for tests
    WHAT’S WRONG WITH SLOW TESTS?

    View full-size slide

  62. You don't run them often
    You waste time waiting for tests
    You distracted others while waiting
    WHAT’S WRONG WITH SLOW TESTS?

    View full-size slide

  63. You don't run them often
    You waste time waiting for tests
    You distracted others while waiting
    WHAT’S WRONG WITH SLOW TESTS?
    http://xkcd.com/303/

    View full-size slide

  64. You don't run them often
    You waste time waiting for tests
    You distracted others while waiting
    You commit failing changes
    WHAT’S WRONG WITH SLOW TESTS?

    View full-size slide

  65. You don't run them often
    You waste time waiting for tests
    You distracted others while waiting
    You commit failing changes
    You lose the rapid feedback cycle
    WHAT’S WRONG WITH SLOW TESTS?

    View full-size slide

  66. unit tests can be slow when they
    INTERACT
    WITH SLOW
    COMPONENTS.

    View full-size slide

  67. context Notifications::Emails::Message do
    setup do
    @comment = Issue.make! # create record in database
    @settings = Notifications::Settings.new(-1)
    @message = Notifications::Emails::Message.new(
    @comment, @settings)
    end
    test "#to uses global email" do
    @settings.email :global, '[email protected]'
    assert_equal '[email protected]', @message.to
    end
    test "#body includes comment body" do
    assert_match @comment.body, @message.body
    end
    end

    View full-size slide

  68. $ ruby test/unit/notifications/emails/message_test.rb
    ...................
    Finished in 3.517926 seconds.
    19 tests, 24 assertions, 0 failures, 0 errors

    View full-size slide

  69. context Notifications::Emails::Message do
    setup do
    @comment = Issue.make # create in memory
    @comment.id = -1 # make it appear persisted
    @settings = Notifications::Settings.new(-1)
    @message = Notifications::Emails::Message.new(
    @comment, @settings)
    end
    test "#to uses global email" do
    @settings.email :global, '[email protected]'
    assert_equal '[email protected]', @message.to
    end
    test "#body includes comment body" do
    assert_match @comment.body, @message.body
    end

    View full-size slide

  70. $ ruby test/unit/notifications/emails/message_test.rb
    ...................
    Finished in 0.073752 seconds.
    19 tests, 24 assertions, 0 failures, 0 errors

    View full-size slide

  71. 3.517926
    ÷ 0.073752
    ~50 X FASTER

    View full-size slide

  72. unit tests can be slow when they
    DON’T TEST
    OBJECTS IN
    ISOLATION.

    View full-size slide

  73. context Notifications::Emails::CommitMention do
    setup do
    @repo = Repository.make!
    readonly_example_repo :notification_mentions, @repo
    @commit = @repo.commit('a62c6b20')
    @comment = CommitMention.new(:commit_id => @commit.sha)
    @message = Emails::CommitMention.new(@comment)
    end
    test 'subject' do
    expected = "[testy] hello world (#{@comment.short_id})"
    assert_equal expected, @message.subject
    end
    end

    View full-size slide

  74. context Notifications::Emails::CommitMention do
    setup do
    @repo = Repository.make!
    readonly_example_repo :notification_mentions, @repo
    @commit = @repo.commit('a62c6b20')
    @comment = CommitMention.new(:commit_id => @commit.sha)
    @message = Emails::CommitMention.new(@comment)
    end
    test 'subject' do
    expected = "[testy] hello world (#{@comment.short_id})"
    assert_equal expected, @message.subject
    end
    end

    View full-size slide

  75. context Notifications::Emails::CommitMention do
    setup do
    @commit = stub(
    :sha => Sham.sha,
    :short_id => '12345678',
    :short_message => 'hello world',
    :message => 'goodbye world'
    )
    @comment = CommitMention.new(:commit_id => @commit.sha)
    @comment.stubs(:commit => @commit)
    @message = Emails::CommitMention.new(@comment)
    end
    test 'subject' do
    expected = "[testy] hello world (#{@comment.short_id})"
    assert_equal expected, message.subject
    end

    View full-size slide

  76. BEFORE
    $ ruby test/unit/notifications/emails/commit_mention_test.rb
    ....
    Finished in 0.576135 seconds.
    AFTER
    $ ruby test/unit/notifications/emails/commit_mention_test.rb
    ....
    Finished in 0.052412 seconds.

    View full-size slide

  77. 0.576135
    ÷ 0.052412
    ~10 X FASTER

    View full-size slide

  78. unit tests can be slow when they
    BOOTSTRAP
    HEAVY
    FRAMEWORKS.

    View full-size slide

  79. $ time ruby test/unit/notifications/email_handler_test.rb

    View full-size slide

  80. $ time ruby test/unit/notifications/email_handler_test.rb
    ........
    Finished in 0.084729 seconds.
    8 tests, 10 assertions, 0 failures, 0 errors

    View full-size slide

  81. $ time ruby test/unit/notifications/email_handler_test.rb
    ........
    Finished in 0.084729 seconds.
    8 tests, 10 assertions, 0 failures, 0 errors
    real 0m7.065s
    user 0m4.948s
    sys 0m1.961s

    View full-size slide

  82. test/fast/notifications/web_handler.rb
    require 'notifications/summary_store'
    require 'notifications/memory_indexer'
    require 'notifications/web_handler'
    context Notifications::WebHandler do
    def web
    @web ||= WebHandler.new(
    :indexer => MemoryIndexer.new,
    :store => SummaryStore.new
    )
    end
    def test_insert_increments_count
    assert_equal 0, web.count(1)
    web.add build_summary, 1
    assert_equal 1, web.count(1)
    end

    View full-size slide

  83. $ time ruby test/fast/notifications/web_handler_test.rb
    ....
    Finished in 0.001577 seconds.
    4 tests, 22 assertions, 0 failures, 0 errors
    real 0m0.139s
    user 0m0.068s
    sys 0m0.063s

    View full-size slide

  84. 7.065
    ÷ 0.139
    ~50 X FASTER

    View full-size slide

  85. SLOW TEST MYTHS

    View full-size slide

  86. SLOW TEST MYTHS
    “Our tests are slow because we have too
    many of them.”

    View full-size slide

  87. SLOW TEST MYTHS
    “Our tests are slow because we have too
    many of them.”
    “To speed up our tests, we just need to
    parallelize them.”

    View full-size slide

  88. SLOW TEST MYTHS
    “Our tests are slow because we have too
    many of them.”
    “To speed up our tests, we just need to
    parallelize them.”
    “We can’t use test doubles because we’ll
    loose confidence that it still works.”

    View full-size slide

  89. PAY ATTENTION
    TO THE SPEED OF
    YOUR TESTS AS
    YOU WRITE THEM.

    View full-size slide

  90. our code smells when
    IT IS TIGHTLY
    COUPLED TO A
    FRAMEWORK.

    View full-size slide

  91. FRAMEWORKS
    ENCOURAGE YOU TO
    PUT ALL OF YOUR
    APPLICATION INSIDE
    THEIR SANDBOX.

    View full-size slide

  92. …WHICH MAKES
    CODE DIFFICULT TO
    TEST, CHANGE AND
    REUSE.

    View full-size slide

  93. God Objects
    class Issue < ActiveRecord::Base

    View full-size slide

  94. God Objects
    class Issue < ActiveRecord::Base
    # validations
    validates_presence_of :title, :user_id, :repository_id

    View full-size slide

  95. God Objects
    class Issue < ActiveRecord::Base
    # validations
    validates_presence_of :title, :user_id, :repository_id
    # associations
    belongs_to :user

    View full-size slide

  96. God Objects
    class Issue < ActiveRecord::Base
    # validations
    validates_presence_of :title, :user_id, :repository_id
    # associations
    belongs_to :user
    # data integrity
    before_validation :set_state

    View full-size slide

  97. God Objects
    class Issue < ActiveRecord::Base
    # validations
    validates_presence_of :title, :user_id, :repository_id
    # associations
    belongs_to :user
    # data integrity
    before_validation :set_state
    # misc concerns
    before_save :audit_if_changed

    View full-size slide

  98. God Objects
    class Issue < ActiveRecord::Base
    # validations
    validates_presence_of :title, :user_id, :repository_id
    # associations
    belongs_to :user
    # data integrity
    before_validation :set_state
    # misc concerns
    before_save :audit_if_changed
    # querying
    named_scope :watched_by, lambda {|user| ... }

    View full-size slide

  99. God Objects
    class Issue < ActiveRecord::Base
    # validations
    validates_presence_of :title, :user_id, :repository_id
    # associations
    belongs_to :user
    # data integrity
    before_validation :set_state
    # misc concerns
    before_save :audit_if_changed
    # querying
    named_scope :watched_by, lambda {|user| ... }
    # who knows what these do?
    include Mentionable, Subscribable, Summarizable

    View full-size slide

  100. God Objects
    class Issue < ActiveRecord::Base
    # validations
    validates_presence_of :title, :user_id, :repository_id
    # associations
    belongs_to :user
    # data integrity
    before_validation :set_state
    # misc concerns
    before_save :audit_if_changed
    # querying
    named_scope :watched_by, lambda {|user| ... }
    # who knows what these do?
    include Mentionable, Subscribable, Summarizable
    # domain logic
    def active_participants
    [self.user] + watchers + commentors
    end
    end

    View full-size slide

  101. MAKE THE FRAMEWORK
    DEPEND ON YOUR
    APPLICATION, INSTEAD
    OF MAKING YOUR
    APPLICATION DEPEND
    ON THE FRAMEWORK.

    View full-size slide

  102. coupled to a
    framework
    GOOS

    View full-size slide

  103. coupled to a
    framework
    the rest of the
    application
    GOOS

    View full-size slide

  104. A typical Rails controller…
    class SessionsController < ApplicationController
    def create
    user = User.authenticate(params[:username],
    params[:password])
    if user
    self.current_user = user
    redirect_to root_path, success: 'You are signed in!'
    else
    render :new, warning: 'Wrong username or password.'
    end
    end
    end

    View full-size slide

  105. …and model
    class User < ActiveRecord::Base
    def self.authenticate(username, password)
    user = find_by_username(username)
    user if user && user.authenticated?(password)
    end
    def authenticated?(password)
    encrypt(password, self.salt) == self.encrypted_password
    end
    def encrypt(password, salt)
    Digest::SHA1.hexdigest(password+salt)
    end
    end

    View full-size slide

  106. Why does this depend on Active Record?
    class User < ActiveRecord::Base
    def self.authenticate(username, password)
    user = find_by_username(username)
    user if user && user.authenticated?(password)
    end
    def authenticated?(password)
    encrypt(password, self.salt) == self.encrypted_password
    end
    def encrypt(password, salt)
    Digest::SHA1.hexdigest(password+salt)
    end
    end

    View full-size slide

  107. The spec is already complex.
    describe User do
    # … a couple hundred lines of specs …
    describe ".authenticate" do
    let!(:user) do
    create :user, :email => "bkeepers", :password => "testing"
    end
    it "returns user with case insensitive username" do
    User.authenticate('BKeepers', 'testing').should == @user
    end
    it "returns nil with incorrect password" do
    User.authenticate("bkeepers", "wrong").should be_nil
    end
    it "returns nil with unknown username" do
    User.authenticate('[email protected]', 'testing').should be_nil
    end
    end
    # … a couple hundred more lines of specs …

    View full-size slide

  108. Create objects to model the domain
    class SessionsController < ApplicationController
    def create
    user = PasswordAuthentication.new(params[:username],
    params[:password]).user
    if user
    self.current_user = user
    redirect_to root_path, success: 'You are signed in!'
    else
    render :new, warning: 'Wrong username or password.'
    end
    end
    end

    View full-size slide

  109. Plain ol’ Ruby class
    class PasswordAuthentication
    def initialize(username, password)
    @username = username
    @password = password
    end
    def user
    end
    end

    View full-size slide

  110. require 'spec_helper'
    describe PasswordAuthentication do
    describe 'user' do
    context 'with a valid username & password'
    context 'with an unknown username'
    context 'with an incorrect password'
    end
    end

    View full-size slide

  111. describe PasswordAuthentication do
    describe 'user' do
    let!(:user) do
    create :user, :username => 'bkeepers',
    :password => 'testing'
    end
    context 'with a valid username & password' do
    subject do
    PasswordAuthentication.new(user.username, 'testing')
    end
    it 'returns the user' do
    subject.user.should == user
    end
    end
    end

    View full-size slide

  112. class PasswordAuthentication
    def initialize(username, password)
    @username = username
    @password = password
    end
    def user
    User.find_by_username(@username)
    end
    end

    View full-size slide

  113. context 'with an unknown username' do
    subject do
    PasswordAuthentication.new('unknown', 'testing')
    end
    it 'returns nil' do
    subject.user.should be_nil
    end
    end

    View full-size slide

  114. No changes necessary
    class PasswordAuthentication
    def initialize(username, password)
    @username = username
    @password = password
    end
    def user
    User.find_by_username(@username)
    end
    end

    View full-size slide

  115. describe PasswordAuthentication do
    describe 'user' do
    context 'with a valid username & password' do # …
    context 'with an unknown username' do # …
    context 'with an incorrect password' do
    subject do
    PasswordAuthentication.new(user.username, 'wrong')
    end
    it 'returns nil' do
    subject.user.should be_nil
    end
    end
    end
    end

    View full-size slide

  116. class PasswordAuthentication
    # …
    def user
    user = User.find_by_username(@username)
    user if user && authenticated?(user)
    end
    private
    def authenticated?(user)
    encrypt(@password, user.password_salt) ==
    user.encrypted_password
    end
    def encrypt(password, salt)
    Digest::SHA1.hexdigest(password+salt)
    end
    end

    View full-size slide

  117. describe PasswordAuthentication do
    describe 'user' do
    let!(:user) do
    create :user, :username => 'bkeepers',
    :password => 'testing'
    # hits the DB :(
    end
    # …
    end
    end

    View full-size slide

  118. describe PasswordAuthentication do
    describe 'user' do
    let!(:user) do
    double :user,
    :username => 'bkeepers',
    :encrypted_password => '…',
    :password_salt => '…'
    end
    before do
    User.stub(:find_by_username).
    with(user.username).
    and_return(user)
    end
    end
    end

    View full-size slide

  119. context 'with an unknown username' do
    before do
    User.should_receive(:find_by_username).
    with('unknown').
    and_return(nil)
    end
    subject do
    PasswordAuthentication.new('unknown', 'testing')
    end
    it 'returns nil' do
    subject.user.should be_nil
    end
    end

    View full-size slide

  120. POSITIVE FEEDBACK LOOP
    Unit tests help us isolate our
    code and reduce coupling to
    frameworks, which makes
    our tests faster.

    View full-size slide

  121. Before we part ways
    EPILOGUE

    View full-size slide

  122. Robert C. Martin
    Clean Code
    “Writing clean code requires the
    disciplined use of a myriad little
    techniques applied through a
    painstakingly acquired sense of
    ‘cleanliness.’”

    View full-size slide

  123. REFERENCES
    Gary Bernhardt
    Fast test, Slow Test
    http://pyvideo.org/video/631/fast-test-slow-test
    Corey Haines
    Fast Rails Tests
    http://confreaks.com/videos/641-gogaruco2011-fast-rails-tests

    View full-size slide

  124. @bkeepers
    http://bit.ly/smells-slides
    QUESTIONS?

    View full-size slide