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

Clean Code Cowboy

Clean Code Cowboy

Have you ever had a fear of writing code? That maybe what you’re about to write makes no sense. Or maybe it’s not following best practices. Maybe it’s full of code smells? Or even worse maybe it doesn’t do what it’s supposed to do? You wrote something and you think “is this right and does it look OK!?”. Do you feel confident about your code? Are you happy with it? Programming is fun but it can be simply stressful - let’s just chill out, write a failing test and make it pass!

Piotr Solnica

October 14, 2014
Tweet

More Decks by Piotr Solnica

Other Decks in Programming

Transcript

  1. – Bob Martin from The Clean Coder book “Programmers who

    endure and succeed amidst swirling uncertainty and nonstop pressure share a common attribute: They care deeply about the practice of creating software. They treat it as a craft. They are professionals.”
  2. http://www.techrepublic.com/blog/10-things/10-types- of-programmers-youll-encounter-in-the-field/ “The Code Cowboy is a force of nature

    that cannot be stopped. He or she is almost always a great programmer and can do work two or three times faster than anyone else. The problem is, at least half of that speed comes by cutting corners.”
  3. describe Calculator do subject(:calculator) { Calculator.new } describe "#sum" do

    it "returns the sum of numbers" do expect(calculator.sum(2, 2)).to be(4) end end end
  4. feature "Home page" do scenario "as a logged-in member I

    can buy add-ons", js: true, vcr: true do expect(page).to have_text("5 PT classes") click_on "Add" expect(page).to have_content("1,000,00 EUR") expect(page).to have_content("Buy") click_on "Buy" expect(page).to have_text("Thanks for buying an add-on") end end Use JavaScript driver Use VCR for stubbed remote calls
  5. feature "Customers" do scenario "As a user I can add

    a new customer and see it on the list" do visit new_customer_path find('#customer_name').set('Big Corp') find('#customer_email').set('[email protected]') find('#customer_address').set('Sunset Street 1') find('input[type=submit]').click expect(page).to have_text("Customer created") within(".customer-list") do expect(page).to have_text("Big Corp") end end end
  6. class CustomersController < ApplicationController def index customers = Customer.all render

    locals: { customers: customers } end def new customer = Customer.new render locals: { customer: customer } end def create attributes = params.require(:customer).permit(:name, :email, :address) Customer.create(attributes) redirect_to :customers, notice: "Customer created" end end
  7. feature "Invoices" do let!(:customer) { Customer.create(name: "Big Corp") } scenario

    "As a user I can create a new invoice and see it on the list" do visit new_invoice_path find("#invoice_customer_id").select("Big Corp") find("#invoice_issue_date").set("2014-10-11") find("#invoice_delivery_date").set("2014-10-11") find("#invoice_due_date").set("2014-11-11") find("input[type=submit]").click expect(page).to have_text("Invoice created") within(".invoice-list") do expect(page).to have_text("DRAFT") expect(page).to have_text("Big Corp") expect(page).to have_text("2014-10-11") expect(page).to have_text("2014-11-11") end end end
  8. class InvoicesController < ApplicationController def index invoices = Invoice.all render

    locals: { invoices: invoices } end def new invoice = Invoice.new customers = Customer.all render locals: { invoice: invoice, customers: customers } end def create attributes = params.require(:invoice).permit( :customer_id, :issue_date, :delivery_date, :due_date, :status ) Invoice.create(attributes) redirect_to :invoices, notice: "Invoice created" end end
  9. feature "Invoices" do scenario "As a user I can create

    a new invoice and see it on the list", js: true do visit new_invoice_path find("#invoice_customer_id").select("Big Corp") find("#invoice_issue_date").set("2014-10-11") find("#invoice_delivery_date").set("2014-10-11") find("#invoice_due_date").set("2014-11-11") find("#line-item-id").select("Development") find("#new-item-qty").set("2") find(“#add-new-item”).click within(".item-list") do expect(page).to have_text("Development") expect(page).to have_text("200.00") end find("input[type=submit]").click expect(page).to have_text("Invoice created") within(".invoice-list") do expect(page).to have_text("DRAFT") expect(page).to have_text("Big Corp") expect(page).to have_text("2014-10-11") expect(page).to have_text("2014-11-11") expect(page).to have_text("200.00") end end end Select line item and provide quantity Make sure the total amount is present Things will happen on the client-side so we need JS Make sure the item was added to the list
  10. $form = $("#new_invoice") $itemSelect = $("#line-item-id") $lineItems = $("#line-items") $addButton

    = $("#add-new-item") $itemQty = $("#new-item-qty") $addButton.on('click', (e) -> e.preventDefault() quantity = $itemQty.val() item = $itemSelect.find(':selected').data() price = parseFloat(item.price) * parseFloat(quantity) $tr = $("<tr/>").append( "<td>#{item.name}</td><td>#{item.unit}</td><td>#{price}</td>" ) $form.append( $("<input/>"). attr("type", "hidden"). attr("name", "invoice[items][][line_item_id]"). val(item.id) ) $form.append( $("<input/>"). attr("type", "hidden"). attr("name", "invoice[items][][quantity]"). val(quantity) ) $lineItems.append($tr) )
  11. When it’s no longer simple to make a test pass

    it’s a good moment to write…more tests
  12. Something must create an invoice along with its line items

    Something must be able to calculate the total amount based on line items
  13. describe CreateInvoice do describe "#call" do it "creates an invoice

    with items" do customer = Customer.create(name: "Big Corp") magic = LineItem.create(name: 'Magic', unit: 'Hour', price: 2) healing = LineItem.create(name: 'Healing', unit: 'Hour', price: 4) today = Date.today params = { status: "DRAFT", customer_id: customer.id, issue_date: today, delivery_date: today, due_date: today + 1.month, items: [ { line_item_id: magic.id, quantity: 2 }, { line_item_id: healing.id, quantity: 3 }, ] } create_invoice = CreateInvoice.new(params) invoice = create_invoice.call expect(invoice.customer).to eql(customer) expect(invoice.line_items.to_a).to eql([magic, healing]) expect(invoice.total_amount).to eql(magic.price * 2 + healing.price * 3) end end end
  14. class CreateInvoice attr_reader :params def initialize(params) @params = params end

    def call items = params[:items] invoice = Invoice.new(params.except(:items)) items.each do |item| invoice.items.build(item) end invoice.save invoice end end
  15. class Invoice < ActiveRecord::Base belongs_to :customer has_many :items, class_name: "InvoiceItem"

    has_many :line_items, through: :items def total_amount items.map { |item| item.line_item.price * item.quantity }.sum end end BUT WAIT A MINUTE!
  16. feature "Invoices" do fixtures(:all) let(:invoice) { invoices(:first) } let(:customer) {

    customers(:big_corp) } scenario "as a user I can display an invoice" do visit invoices_path click_on invoice.number expect(page).to have_content(customer.name) expect(page).to have_content(invoice.number) expect(page).to have_content(invoice.issue_date) expect(page).to have_content(invoice.delivery_date) expect(page).to have_content(invoice.due_date) expect(page).to have_content(invoice.total_amount) end end missing interface!
  17. class Invoice < ActiveRecord::Base belongs_to :customer has_many :items, class_name: "InvoiceItem"

    has_many :line_items, through: :items # TODO: implement me alias_method :number, :id def total_amount items.map { |item| item.line_item.price * item.quantity }.sum end end there we go, all good
  18. feature "Invoices" do fixtures(:all) let(:invoice) { invoices(:first) } let(:customer) {

    customers(:big_corp) } scenario "as a user I can display an invoice" do visit invoices_path click_on invoice.number expect(page).to have_content(customer.name) expect(page).to have_content(invoice.number) expect(page).to have_content(invoice.issue_date) expect(page).to have_content(invoice.delivery_date) expect(page).to have_content(invoice.due_date) expect(page).to have_content(invoice.total_amount) invoice.items.each do |item| expect(page).to have_content(item.line_item.name) expect(page).to have_content(item.line_item.price * item.quantity) end end end Demeter no likey :(
  19. h1 = "#{invoice.id}" table thead tr th Customer th Issue

    date th Delivery date th Due date th Total amount tbody tr td = invoice.customer.name td = invoice.issue_date td = invoice.delivery_date td = invoice.due_date td = number_to_currency invoice.total_amount ul - invoice.items.each do |item| li = "#{item.line_item.name} #{number_to_currency(item.line_item.price * item.quantity)}" I told you mate but you wouldn’t listen!
  20. describe InvoiceItem do fixtures(:invoice_items, :line_items) subject(:invoice_item) { invoice_items(:one) } describe

    "#price" do let(:line_item) { line_items(:magic) } it "returns the price based on quantity and line-item price" do expect(invoice_item.price).to eql(line_item.price * invoice_item.quantity) end end end
  21. class InvoiceItem < ActiveRecord::Base belongs_to :invoice belongs_to :line_item def price

    line_item.price * quantity end end class Invoice < ActiveRecord::Base # stuff def total_amount items.to_a.sum(&:price) end end