How To Code Like A Writer

3
How to Code
Like a Writer
Nickolas Means

View Slide

Hi, I’m Nick.
(@nmeans if you’re on “the twitters”)
"

View Slide

(hint: you’re in this town right now)

View Slide

I build stuff at
to help fix healthcare.

View Slide

View Slide

“If we're not
engineers, what are
we then? We're
software writers!”
– David Heinemer Hansson
2014 Railsconf Keynote

View Slide

View Slide

And now, some code.

View Slide

Keep Ruby Weird,
meet
shipping_utilities.rb

View Slide

module ShippingUtilities
class ShippingException < Exception; end
# Struct classes
ShippingGroup = Struct.new( :order_line_items, :vendor, :shipping_cost, :applied_shipping_allowance ) unless const_defined?(:ShippingGroup)
ShippingDisplayOption = Struct.new( :value, :text ) unless const_defined?(:ShippingDisplayOption)
CHOOSE_COUNTRY_FIRST = [
ShippingDisplayOption.new( "", "Please choose destination country first" )
] unless const_defined?("CHOOSE_COUNTRY_FIRST")
# PO Box or Domestic
FCM_DOMESTIC_SHIPPING_OPTION = [
ShippingDisplayOption.new( "usps_fcm", "USPS First Class Mail")
] unless const_defined?("FCM_DOMESTIC_SHIPPING_OPTION")
FREE_LIGHTWEIGHT_MAIL_OPTION = [
ShippingDisplayOption.new( "free_usps_fcm", "USPS First Class Mail - FREE!")
] unless const_defined?("DISCOUNT_LIGHTWEIGHT_MAIL_OPTION")
# Domestic
DOMESTIC_COMMON_SHIPPING_OPTIONS = [
ShippingDisplayOption.new( "next_business_day", "Next Business Day" ),
ShippingDisplayOption.new( "two_day", "Second Business Day" ),
ShippingDisplayOption.new( "three_day", "Third Business Day" )
] unless const_defined?("DOMESTIC_COMMON_SHIPPING_OPTIONS")
STANDARD_GROUND_OPTION = [
ShippingDisplayOption.new( "ground", "Standard Ground" )
] unless const_defined?("STANDARD_GROUND_OPTION")
FREE_GROUND_OPTION = [
ShippingDisplayOption.new( "free_ground", "Standard Ground - FREE!" )
] unless const_defined?("FREE_GROUND_OPTION")
STANDARD_MAIL_OPTION = [
ShippingDisplayOption.new( "usps_domestic", "USPS Priority Mail")
] unless const_defined?("STANDARD_MAIL_OPTION")
DISCOUNT_MAIL_OPTION = [
ShippingDisplayOption.new( "discounted_usps_domestic", "USPS Priority Mail - DISCOUNTED!")
] unless const_defined?("DISCOUNT_MAIL_OPTION")
# LOWER 48
FS_PROMO_DOMESTIC_SHIPPING_OPTIONS =
DOMESTIC_COMMON_SHIPPING_OPTIONS +
FREE_GROUND_OPTION
FS_PROMO_LIGHTWEIGHT_SHIPPING_OPTIONS =
STANDARD_GROUND_OPTION +
FREE_LIGHTWEIGHT_MAIL_OPTION
STANDARD_DOMESTIC_SHIPPING_OPTIONS =
STANDARD_GROUND_OPTION
LIGHTWEIGHT_DOMESTIC_SHIPPING_OPTIONS =
STANDARD_DOMESTIC_SHIPPING_OPTIONS +
FCM_DOMESTIC_SHIPPING_OPTION
PO_BOX_DOMESTIC_SHIPPING_OPTIONS =
STANDARD_MAIL_OPTION
LIGHTWEIGHT_PO_BOX_DOMESTIC_SHIPPING_OPTIONS =
STANDARD_MAIL_OPTION +
# AK/HI
FS_PROMO_AK_HI_SHIPPING_OPTIONS =
DISCOUNT_MAIL_OPTION
FS_PROMO_LIGHTWEIGHT_AK_HI_SHIPPING_OPTIONS =
STANDARD_AK_HI_SHIPPING_OPTIONS =
LIGHTWEIGHT_AK_HI_SHIPPING_OPTIONS =
STANDARD_AK_HI_SHIPPING_OPTIONS +
# TERRITORIES
FS_PROMO_TERRITORIES_SHIPPING_OPTIONS =
DISCOUNT_MAIL_OPTION
FS_PROMO_LIGHTWEIGHT_TERRITORIES_SHIPPING_OPTIONS =
STANDARD_TERRITORIES_SHIPPING_OPTIONS =
LIGHTWEIGHT_TERRITORIES_SHIPPING_OPTIONS =
# International
COMMON_INTERNATIONAL_SHIPPING_OPTIONS = [
ShippingDisplayOption.new( "ups_express", "UPS Worldwide Express [1-3 days]" ),
ShippingDisplayOption.new( "ups_expedited", "UPS Worldwide Expedited [2-5 days]" ),
ShippingDisplayOption.new( "usps_international", "USPS Express Mail International")
] unless const_defined?("INTERNATIONAL_SHIPPING_OPTIONS")
DISCOUNTED_INTERNATIONAL_PMI_OPTION = [
ShippingDisplayOption.new( "discounted_usps_int_priority", "USPS Priority Mail International - DISCOUNTED!")
] unless const_defined?("DISCOUNTED_INTERNATIONAL_PMI_OPTION")
STANDARD_INTERNATIONAL_PMI_OPTION = [
ShippingDisplayOption.new( "usps_int_priority", "USPS Priority Mail International")
] unless const_defined?("STANDARD_INTERNATIONAL_PMI_OPTION")
FS_PROMO_INTERNATIONAL_SHIPPING_OPTIONS =
COMMON_INTERNATIONAL_SHIPPING_OPTIONS +
DISCOUNTED_INTERNATIONAL_PMI_OPTION
STANDARD_INTERNATIONAL_SHIPPING_OPTIONS =
COMMON_INTERNATIONAL_SHIPPING_OPTIONS +
STANDARD_INTERNATIONAL_PMI_OPTION
ALL_SHIPPING_OPTIONS = DOMESTIC_COMMON_SHIPPING_OPTIONS + FREE_GROUND_OPTION + STANDARD_GROUND_OPTION + COMMON_INTERNATIONAL_SHIPPING_OPTIONS + DISCOUNTED_INTERNATIONAL_PMI_OPTION +
STANDARD_INTERNATIONAL_PMI_OPTION + FCM_DOMESTIC_SHIPPING_OPTION + FREE_LIGHTWEIGHT_MAIL_OPTION + DISCOUNT_MAIL_OPTION + STANDARD_MAIL_OPTION unless const_defined?("ALL_SHIPPING_OPTIONS")
INTERNATIONAL_SHIPPING_METHODS = [
ShippingMethod.new( :shipping_option => :usps_int_priority, :api => :usps, :name => "USPS Priority Mail International", :mail_class => "PriorityMailInternational", :multiplier => 1.45),
ShippingMethod.new( :shipping_option => :usps_international, :api => :usps, :name => "USPS Express Mail International", :mail_class => "ExpressMailInternational", :multiplier => 1.45),
ShippingMethod.new( :shipping_option => :ups_express, :api => :ups, :name => "UPS Worldwide Express", :service_code => "07", :multiplier => 0.7),
ShippingMethod.new( :shipping_option => :ups_expedited, :api => :ups, :name => "UPS Worldwide Expedited", :service_code => "08", :multiplier => 0.7)
] unless const_defined?("INTERNATIONAL_SHIPPING_METHODS")
DOMESTIC_FEDEX_SHIPPING_METHODS = [
ShippingMethod.new( :shipping_option => :ground, :api => :fedex, :name => "FedEx Ground", :transaction_type => "rate_ground", :service_type => "ground_service", :multiplier => 1.4 ),
ShippingMethod.new( :shipping_option => :ground_res, :api => :fedex, :name => "FedEx Home Delivery", :transaction_type => "rate_ground", :service_type => "home_delivery", :multiplier => 1.4),
ShippingMethod.new( :shipping_option => :three_day, :api => :fedex, :name => "FedEx Express Saver (3 Day)", :transaction_type => "rate_express", :service_type => "express_saver", :multiplier => 1.25 ),
ShippingMethod.new( :shipping_option => :two_day, :api => :fedex, :name => "FedEx 2 Day", :transaction_type => "rate_express", :service_type => "2day", :multiplier => 1.25 ),
ShippingMethod.new( :shipping_option => :next_business_day, :api => :fedex, :name => "FedEx Standard Overnight", :transaction_type => "rate_express", :service_type => "standard_overnight", :multiplier => 1.25 )
] unless const_defined?("DOMESTIC_FEDEX_SHIPPING_METHODS")
DOMESTIC_UPS_SHIPPING_METHODS = [
ShippingMethod.new( :shipping_option => :ground, :api => :ups, :name => "UPS Ground", :service_code => "03"),
ShippingMethod.new( :shipping_option => :three_day, :api => :ups, :name => "UPS 3-Day Select", :service_code => "12", :multiplier => 1.1),
ShippingMethod.new( :shipping_option => :two_day, :api => :ups, :name => "UPS 2nd Day Air", :service_code => "02", :multiplier => 1.1),
ShippingMethod.new( :shipping_option => :next_business_day, :api => :ups, :name => "UPS Next Day Air", :service_code => "01", :multiplier => 1.1)
] unless const_defined?("DOMESTIC_UPS_SHIPPING_METHODS")
DOMESTIC_USPS_SHIPPING_METHOD = [
ShippingMethod.new( :shipping_option => :upsmi_domestic, :api => :upsmi, :name => "UPS Mail Innovations", :service => "domestic"),
ShippingMethod.new( :shipping_option => :usps_fcm, :api => :usps, :name => "USPS First Class Mail", :mail_class => "FIRST"),
ShippingMethod.new( :shipping_option => :usps_domestic, :api => :usps, :name => "USPS Priority Mail", :mail_class => "PRIORITY")
] unless const_defined?("DOMESTIC_USPS_SHIPPING_METHOD")
ALL_SHIPPING_METHODS = INTERNATIONAL_SHIPPING_METHODS + DOMESTIC_FEDEX_SHIPPING_METHODS + DOMESTIC_UPS_SHIPPING_METHODS + DOMESTIC_USPS_SHIPPING_METHOD
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
# Lookup the ShippingMethod object based on the shipping code passed into
# the method here.
#
# Note that shipping_code will be one of the symbols based on the
# :shipping_option => 'XXXXX' in the above declared ShippingMethod instances.
#
# Therefore shipping_code will be something like:
# "ground", "three_day", "two_day", "next_business_day", "usps_domestic", etc.
#
#
# shipping_code will be a STRING... NOT A SYMBOL!
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
def lookup_shipping_method( shipping_code, api_override = false )
# Try domestic shipping first. For domestic we need to match based on the
# shipping code ***AND*** the currently configured shipping API - which
# will be either Fedex or UPS
#RAILS_DEFAULT_LOGGER.debug "Looking up shipping method based on this code: #{shipping_code}."
# Check to see what the currently configured shipping method is - Set by an administrator
# in the admin interface. The only valid values are "ups" and "fedex"
if api_override
api = api_override
else
# Default shipping method is UPS
api = "ups"
end
case api
when "ups"
shipping_method = get_shipping_method_from_array( shipping_code, DOMESTIC_UPS_SHIPPING_METHODS)
when "fedex"
shipping_method = get_shipping_method_from_array( shipping_code, DOMESTIC_FEDEX_SHIPPING_METHODS)
end
# ...if it turns out that we found an appropriate UPS or Fedex method from above - return it...
return shipping_method unless shipping_method.nil?
# ...otherwise lets see if it is a USPS domestic
if ["usps_domestic", "upsmi_domestic", "usps_fcm"].include? shipping_code
return get_shipping_method_from_array( shipping_code, DOMESTIC_USPS_SHIPPING_METHOD )
end
# ... otherwise it must be]an international
return get_shipping_method_from_array( shipping_code, INTERNATIONAL_SHIPPING_METHODS)
end
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
# Get the generic shipping method currently selected
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
def lookup_generic_shipping_method( shipping_code )
ALL_SHIPPING_OPTIONS.each do |opt|
if opt.value == shipping_code
return opt
end
end
return nil
end
def lookup_generic_shipping_method_by_name( method_name )
ALL_SHIPPING_METHODS.each do |opt|
if opt.sm_params[:name] == method_name
return opt.sm_params[:shipping_option]
end
end
return nil
end
def lookup_shipping_method_by_name( method_name )
ALL_SHIPPING_METHODS.each do |opt|
if opt.sm_params[:name] == method_name
return opt
end
end
return nil
end
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
# Get the total shipping cost for the order.
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
def get_shipping_cost_for_order( all_items,
receiver_zip,
receiver_country,
in_stock_shipping_method_code,
out_of_stock_shipping_method_code,
consolidate = false )
raise Exception.new( "ShippingUtilities::get_shipping_cost_for_order is deprecated! Please use new ShippingUtilities::build_shipping_groups instead." )
end
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
# Determine the shipping weight for some items
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
def shipping_weight_for_items(order_line_items)
weight = 0.0
order_line_items.each { |order_line_item|
unless order_line_item.item.shipping_weight.nil? or order_line_item.item.shipping_weight == 0
weight += (order_line_item.quantity * order_line_item.item.shipping_weight)
else
weight += (order_line_item.quantity * 5)
end
}
return weight
end
private
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
# Get a shipping method from a narrowed down array of options
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
def get_shipping_method_from_array( code, methods )
#RAILS_DEFAULT_LOGGER.debug "Code is: #{code}. Methods are: #{methods.inspect}"
methods.each do |shipping_method|
#RAILS_DEFAULT_LOGGER.debug "Shipping Method Compare: #{shipping_method.sm_params[:shipping_option]}"
if shipping_method.sm_params[:shipping_option].to_s == code
#RAILS_DEFAULT_LOGGER.debug "Returning Shipping Method: #{shipping_method.inspect}"
return shipping_method
end
end
return nil
end
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
# Technically the shipping utilities shouldn't have any awareness of what store
# to bill stuff to - but given that we need some "special" logic to determine
# what stores shipping credentials to use depending on what items are in the cart
# this seems like the best option.
#
# If the cart has any NEML products - then use NEML
# If the card is mixed up of NEMX and NEHP only - then use NEMX
# Otherwise use whatever store that all products are from
#
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
def determine_store_for_shipping( all_items )
return Store::NEML if all_items.length < 1
stores = Store.stores_for_items( all_items.collect{ |order_line_item| order_line_item.item || order_line_item.used_item } )
if stores.length == 1
return stores.values.first
else
if stores[Store::NEML] != nil
return stores[Store::NEML]
else
return stores[Store::NEHP]
end
end
end
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
# Calculate the shipping costs for a group of products.
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
def calculate_shipping_costs_for_items( store, order_line_items, sender_zip, receiver_zip, receiver_country, shipping_method_code, consolidate = false )
# Before doing anything, check the validity of the zip code if we're shipping to the US.
# Raise an exception if it's bunk.
if receiver_country == "US"
ShippingCalculatorBase.state_from_zip( receiver_zip )
end
shipping_weight = shipping_weight_for_items( order_line_items )
ship_symbol = consolidate ? :order_in : order_line_items[0].item.shipping_status_symbol
# Prevent exception if sender_zip is not defined (eg. zip not in Vendor table)
if sender_zip.nil?
sender_zip = 79363
end
RAILS_DEFAULT_LOGGER.info "Shipping symbol - #{ship_symbol}"
if ship_symbol == :drop_ship
api_override = "ups"
else
api_override = false
end
RAILS_DEFAULT_LOGGER.info "Method: #{shipping_method_code}, Override: #{api_override}, Store: #{store.id}"
shipping_method = lookup_shipping_method( shipping_method_code, api_override )
RAILS_DEFAULT_LOGGER.info "We found: #{shipping_method.inspect}"
ship_api = shipping_method.sm_params[:api]
# If we don't have an explicit multiplier, we want to use the price
# returned from the API as is (so mult by 1.0)
# DEPRECATED - NEW CODE BELOW!
# shipping_method.sm_params[:multiplier] ||= 1.0
mult = Multiplier.find(:first, :conditions => ["store_id = ? and service = ?", store.id, shipping_method_code], :select => "ds, wh").send((ship_symbol == :drop_ship && receiver_country == "US") ? "ds" : "wh")
min = store.send(ship_symbol == :drop_ship ? "ds_min" : "wh_min")
ship_params = Hash.new
ship_params[:shipping_weight] = shipping_weight
ship_params[:sender_zip] = sender_zip
ship_params[:receiver_zip] = receiver_zip
ship_params[:receiver_country] = receiver_country
case ship_api
when :fedex
ship_params[:fedex_account] = store.fedex_account
ship_params[:fedex_url] = NewEnough::FEDEX_URL
ship_params[:fedex_meter] = store.fedex_meter
ship_params[:service_type] = shipping_method.sm_params[:service_type]
ship_params[:transaction_type] = shipping_method.sm_params[:transaction_type]
amt = (FedexCalculator.new.price( ship_params ) * mult).prec(2)
when :ups
ship_params[:ups_user] = NewEnough::UPS_USER
ship_params[:ups_pass] = NewEnough::UPS_PASS
ship_params[:ups_license] = NewEnough::UPS_LICENSE
ship_params[:sender_country] = store.country
ship_params[:sender_phone] = store.phone_number
ship_params[:sender_name] = store.name
ship_params[:receiver_country] = receiver_country
ship_params[:shipping_method] = shipping_method.sm_params[:service_code]
amt = (UpsCalculator.new.price( ship_params ) * mult).prec(2)
when :usps
ship_params[:mail_class] = shipping_method.sm_params[:mail_class]
ship_params[:partner_id] = NewEnough::ENDICIA_PARTNER_ID
ship_params[:account_id] = NewEnough::ENDICIA_ACCOUNT_ID
ship_params[:pass_phrase] = NewEnough::ENDICIA_PASS_PHRASE
amt = (UspsCalculator.new.price( ship_params ) * mult).prec(2)
when :upsmi
ship_params[:service] = shipping_method.sm_params[:service]
amt = (UpsmiCalculator.new.price( ship_params ) * mult).prec(2)
end
return amt > min ? amt : min
end
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
# Determine the groups in which items will be shipped. This is necessary for
# non-consolidated shipments when some items are out of stock or are being
# drop-shipped from a vendors warehouse.
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
def build_shipments( order_line_items, consolidate=false )
shipments = []
unless consolidate
# Build a hash of line items with associated data we can use without having to re-query the database for all the iterations.
line_items = []
order_line_items.each do |order_line_item|
item = order_line_item.item_id ? Item.find(order_line_item.item_id, :include => :product) : UsedItem.find(order_line_item.used_item_id)
RAILS_DEFAULT_LOGGER.info("Build shipments found item: #{item.inspect} from store #{item.store_id}")
ship_status_symbol = item.shipping_status_symbol_for_quantity(order_line_item.quantity)
ship_status_symbol = :in_stock if [:closeout_instock,:from_stock_only_instock].include?(ship_status_symbol)
line_items << OpenStruct.new( :ship_status_symbol => ship_status_symbol,
:store_id => item.instance_of?(Item) ? item.product.store_id : 1,
:vendor_id => item.instance_of?(Item) ? item.product.vendor_id : nil,
:line_item => order_line_item,
:drop_shippable => item.drop_shippable?,
:consolidatable => false)
end
# MAGIC GOES HERE TO DO THE CONSOLIDATION
line_items_by_sym = {}
[:in_stock,:drop_ship,:order_in].each do |ship_sym|
line_items_by_sym[ship_sym] = line_items.select{|li| li.ship_status_symbol == ship_sym}
end
if line_items_by_sym[:in_stock].length > 0 && line_items_by_sym[:drop_ship].length > 0
RAILS_DEFAULT_LOGGER.debug("Attempting to consolidate in_stocks and drop_ships.")
# If we've got us some in_stocks and some drop_ships, let's see if
# we can do some consolidating. It only makes sense to consolidate if we can completely
# get rid of in_stocks.
line_items_by_sym[:in_stock].each do |li|
matching_ds = line_items_by_sym[:drop_ship].count{|dsli| dsli.vendor_id == li.vendor_id} > 0
li.consolidatable = (matching_ds && li.drop_shippable)
end
if line_items_by_sym[:in_stock].count{|li| !li.consolidatable} < 1
RAILS_DEFAULT_LOGGER.debug("Consolidating in_stocks and drop_ships.")
#Woo-hoo! Let's consolidate
line_items.each{|li| li.ship_status_symbol = :drop_ship if li.ship_status_symbol == :in_stock}
end
end
line_items.each do |li|
create_group_if_necessary_and_insert( shipments, li.ship_status_symbol, li.store_id,
li.ship_status_symbol == :drop_ship ? li.vendor_id : li.store_id, li.line_item )
end
else
create_group_if_necessary_and_insert( shipments, :consolidated, item.store_id, item.store_id, order_line_item)
end
end
set_consolidate_flags( shipments )
RAILS_DEFAULT_LOGGER.info "====================================================="
RAILS_DEFAULT_LOGGER.info "Shipments"
RAILS_DEFAULT_LOGGER.info "#{shipments.inspect}"
return shipments
end
def set_consolidate_flags( shipments )
#RAILS_DEFAULT_LOGGER.debug shipments.to_yaml
# Now let's set appropriate consolidate flags on the shipments that are to be shipped together (NEML & NEHP)
in_stock = []
order_in = []
consol = []
shipments.each do |s|
if s.shipment_type == Shipment::IN_STOCK
in_stock << s
elsif s.shipment_type == Shipment::ORDER_IN
order_in << s
elsif s.shipment_type == Shipment::CONSOLIDATED
consol << s
end
end
# NEMX_TODO - Make this more robust if we need to combine with NEMX in the future.
groups = [in_stock, order_in, consol]
groups.each do |g|
#RAILS_DEFAULT_LOGGER.debug "================================================================"
#RAILS_DEFAULT_LOGGER.debug "Group #{g.inspect}"
#RAILS_DEFAULT_LOGGER.debug "================================================================"
if g.length == 2
# We can assume here that shipper_id == store_id always because a drop-ship from one store
# will never be combined with a drop-ship from another store. If shipment was drop-shipped
# shipper_id == vendor_id.
if g[1].shipper_id == 1
g[0].combine_flag = "combine"
g[1].combine_flag = "has_combined_shipment"
g[1].combine_with_shipment = g[0]
else
g[1].combine_flag = "combine"
g[0].combine_flag = "has_combined_shipment"
g[0].combine_with_shipment = g[1]
end
end
end
end
def create_group_if_necessary_and_insert( shipments, key, store_id, shipper_id, order_line_item)
found = false
shipments.each do |shipment|
if shipment.shipment_type == shipment.shipment_type_from_symbol( key ) && shipment.shipper_id == shipper_id && shipment.store_id == store_id
shipment.line_items << order_line_item
found = true
end
end
if found == false
shipment = Shipment.new
shipment.shipment_type = shipment.shipment_type_from_symbol( key )
shipment.shipper_id = shipper_id
shipment.store_id = store_id
shipments << shipment
end
end
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
# If shipments will be combined, consolidate them for the public view
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
def build_shipping_groups( shipments, ship_method, customer_zip, customer_country, consolidate = false )
RAILS_DEFAULT_LOGGER.debug "In build_shipping_groups, ship_method: #{ship_method}"
shipping_groups = {}
unless shipment.combine_flag == "combine"
@group_line_items = []
shipment.line_items.each {|li| @group_line_items << li }
if !shipment.combine_with_shipment.nil?
shipment.combine_with_shipment.line_items.each {|cli| @group_line_items << cli }
end
ship_symbol = consolidate ? :order_in : @group_line_items[0].item.shipping_status_symbol
shipping_weight = shipping_weight_for_items( @group_line_items )
if ship_method == 'free_ground' && !shipment.has_oversized_item?
if ship_symbol == :drop_ship
shipment.shipping_method = 'ground'
RAILS_DEFAULT_LOGGER.debug("Drop Ship")
elsif shipping_weight <= 5.0
# This is where the UPS Basic selection will kick in once we're approved.
# TODO: UPS Basic
RAILS_DEFAULT_LOGGER.debug("< 5#")
else
RAILS_DEFAULT_LOGGER.debug("Normal")
end
elsif ship_method == 'free_ground' && shipment.has_oversized_item?
elsif ship_method == 'discounted_usps_domestic'
shipment.shipping_method = 'usps_domestic'
elsif ship_method == 'discounted_usps_int_priority'
shipment.shipping_method = 'usps_int_priority'
elsif ship_method == 'free_usps_fcm'
shipment.shipping_method = 'usps_fcm'
else
shipment.shipping_method = ship_method
end
shipment.shipping_cost = calculate_shipping_costs_for_items( determine_store_for_shipping(@group_line_items), @group_line_items, shipment.ships_from.zip_code, customer_zip, customer_country, shipment.shipping_method, consolidate)
if ship_method == 'free_ground'
shipment.applied_shipping_allowance = shipment.has_oversized_item? ? 0.0 : shipment.shipping_cost
elsif ship_method == 'free_usps_fcm'
shipment.applied_shipping_allowance = shipment.shipping_cost
elsif ['discounted_usps_int_priority', 'discounted_usps_domestic'].include?(ship_method)
shipment.applied_shipping_allowance = equivalent_ship_discount_for_weight( shipping_weight )
shipment.applied_shipping_allowance = shipment.shipping_cost if shipment.applied_shipping_allowance > shipment.shipping_cost
else
shipment.applied_shipping_allowance = 0.0
end
if shipment.shipment_type == Shipment::DROP_SHIP
vendor = shipment.ships_from
key = "#{vendor.id}-#{shipment.store_id}"
else
vendor = nil
key = shipment.shipment_type_symbol
end
add_key_if_necessary_and_insert( shipping_groups, key, shipment, @group_line_items, vendor)
end # unless
end
RAILS_DEFAULT_LOGGER.info "Shipping Groups"
RAILS_DEFAULT_LOGGER.info "#{shipping_groups.inspect}"
return shipping_groups
end
def add_key_if_necessary_and_insert( shipping_groups, key, shipment, order_line_items, vendor )
if shipping_groups[ key ].nil?
shipping_groups[key] = ShippingGroup.new( [], vendor, shipment.shipping_cost, shipment.applied_shipping_allowance )
end
order_line_items.each do |item|
shipping_groups[key].order_line_items << item
end
end
def equivalent_ship_discount_for_weight( weight_in_pounds )
# Base of $6.53 covers shipments up to 1 lb.
# We need to figure out rounded-up weight over 1 lb.
additional_poundage = weight_in_pounds - 1
ship_discount = 6.53 + (additional_poundage * 0.29)
# Now we add in the ground multiplier so the rate is realistic
# if someone plays looky-loo with our domestic rates
mult = Multiplier.find(:first, :conditions => {:store_id => 1, :service => 'ground'}).wh
return (ship_discount * mult).prec(2)
end
end

View Slide

View Slide

Some Statistics
• 651 lines of code, 427 SLOC
• Flog score: 561.1
nmeans@nicksair% flog app/models/shipping_utilities.rb
561.1: flog total
35.1: flog/method average
120.7: ShippingUtilities#build_shipments
98.0: ShippingUtilities#build_shipping_groups
96.9: ShippingUtilities#none
88.4: ShippingUtilities#calculate_shipping_costs_for_items
• Flay score: 70
• CodeClimate Grade: F

View Slide

# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
shipments = []
unless consolidate
line_items = []
RAILS_DEFAULT_LOGGER.info(“Line Items: #{order_line_items.inspect}”)
end
end
end
end
end
end
else
end
end
return shipments
end

View Slide

View Slide

“We'll take some smelly
Ruby and refactor it
using only advice from
Strunk and White's  
The Elements of Style.”
– Some Idiot
2014 KRW Talk Proposal

View Slide

What, then, shall we do?
V

View Slide

Strunk and White
5.5 Revise and rewrite
Revising is part of writing. Few writers
are so expert that they can produce what
they are after on the first try. Quite often
you will discover, on examining the
completed work, that there are serious
flaws in the arrangement of the material,
calling for transpositions.

View Slide

# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
shipments = []
unless consolidate
line_items = []
end
end
end
end
end
end
else
end
end
return shipments
end

View Slide

Strunk and White
5.3 Work from a suitable design
Before beginning to compose something,
gauge the nature and extent of the
enterprise and work from a suitable
design.

View Slide

In Stock!
Vendor Foo
Shopping Cart

View Slide

In Stock!
Vendor Foo
Drop Ship
Vendor Foo
Shopping Cart

View Slide

In Stock!
Vendor Foo
Drop Ship
Vendor Foo
Drop Ship
Vendor Bar
Shopping Cart

View Slide

Shopping Cart
In Stock!
Vendor Foo
Drop Ship
Vendor Foo
Drop Ship
Vendor Bar

View Slide

# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
shipments = []
unless consolidate
line_items = []
end
end
end
end
end
end
else
end
end
return shipments
end

View Slide

# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
...
end

View Slide

class ShipmentBuilder
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
def initialize(order_line_items, consolidate = false)
build_shipments(order_line_items, consolidate)
end
def build_shipments( order_line_items, consolidate)
...
end
found = false
if shipment.shipment_type == key && shipment.shipper_id == shipper_id && shipment.store_id == store_id
found = true
end
end
if found == false
shipment.shipment_type = key
end
end
end

View Slide

require_relative 'spec_helper'
require_relative 'shipment_builder'
require 'ostruct'
describe ShipmentBuilder do
it "exists" do
assert ShipmentBuilder.new([order_line_item_double])
end
end
def order_line_item_double
OpenStruct.new(:item_id => 1)
end

View Slide

require 'ostruct'
it "exists" do
end
end
end
nmeans@nicksair% ruby shipment_builder_spec.rb
Run options: --seed 45718
# Running:

Finished in 0.001471s, 679.8097 runs/s, 0.0000 assertions/s.
1) Error:
ShipmentBuilder#test_0001_exists:
NameError: uninitialized constant ShipmentBuilder::RAILS_DEFAULT_LOGGER
1 runs, 0 assertions, 0 failures, 1 errors, 0 skips

View Slide

# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
shipments = []
unless consolidate
line_items = []
end
end
end
end
end
end
else
end
end
return shipments
end

View Slide

# Running:

1) Error:
ShipmentBuilder#test_0001_exists:
NameError: uninitialized constant ShipmentBuilder::Item

View Slide

# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
shipments = []
unless consolidate
line_items = []
end
end
end
end
end
end
else
end
end
return shipments
end

View Slide


View Slide

Strunk and White
5.19 Do not take shortcuts at the cost of clarity
Many shortcuts are self-defeating; they
waste the reader’s time instead of
conserving it. The one truly reliable
shortcut in writing is to choose words
that are strong and surefooted to carry
readers on their way.

View Slide

# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
shipments = []
unless consolidate
line_items = []
end
end
end
end
end
end
else
end
end
return shipments
end

View Slide

line_items = []
end

View Slide

line_items = []
item = if order_line_item.item_id
Item.find(order_line_item.item_id, :include => :product)
else
UsedItem.find(order_line_item.used_item_id)
end
end

View Slide

require 'ostruct'
it "exists" do
end
end
end
"
line_items = []
else
end
end

View Slide

require 'ostruct'
it "exists" do
end
end
OpenStruct.new(:item_id => 1, :store_id => 1)
end
"
line_items = []
else
end
end

View Slide

require 'ostruct'
it "exists" do
end
end
OpenStruct.new(:item_id => 1, :store_id => 1)
end
"
line_items = []
else
end
:store_id => order_line_item.store_id,
end

View Slide

require 'ostruct'
it "exists" do
end
end
OpenStruct.new(:item_id => 1, :store_id => 1, :vendor_id => 1)
end
"
line_items = []
else
end
end

View Slide

require 'ostruct'
it "exists" do
end
end
end
"
line_items = []
else
end
:vendor_id => order_line_item.vendor_id,
end

View Slide

Strunk and White
2.19 Express coordinate ideas in similar form
This principle, that of parallel
construction, requires that expressions
similar in content and function be
outwardly similar. The likeness of form
enables the reader to recognize more
readily the likeness of content and
function.

View Slide

require 'ostruct'
it "exists" do
end
end
end
"
line_items = []
else
end
end

View Slide

require 'ostruct'
it "exists" do
end
end
OpenStruct.new(:item_id => 1, :store_id => 1, :vendor_id => 1, :drop_shippable => true)
end
"
line_items = []
else
end
end

View Slide

require 'ostruct'
it "exists" do
end
end
OpenStruct.new(:item_id => 1, :store_id => 1, :vendor_id => 1, :drop_shippable => true)
end
"
line_items = []
else
end
:drop_shippable => order_line_item.drop_shippable,
end

View Slide

require 'ostruct'
it "exists" do
end
end
OpenStruct.new(:item_id => 1, :store_id => 1, :vendor_id => 1, :drop_shippable? => true)
end
"
line_items = []
else
end
:drop_shippable => order_line_item.drop_shippable?,
end

View Slide

require 'ostruct'
it "exists" do
end
end
OpenStruct.new(:item_id => 1, :store_id => 1, :vendor_id => 1, :drop_shippable? => true,
:ship_status_symbol => :in_stock)
end
"
line_items = []
else
end
end

View Slide

require 'ostruct'
it "exists" do
end
end
end
"
line_items = []
else
end
line_items << OpenStruct.new( :ship_status_symbol => order_line_item.ship_status_symbol,
end

View Slide

require 'ostruct'
it "exists" do
end
end
end
"
line_items = []
else
end
end

View Slide

line_items = []
else
end
end

View Slide

line_items = []
end

View Slide

shipments = []
unless consolidate
line_items = []
end
end
end
end
end
end
else
item = order_line_item.item_id ? Item.find(order_line_item.item_id, :include => :product) :
end
end
return shipments
end

View Slide

# Running:


View Slide

# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
shipments = []
unless consolidate
line_items = []
end
end
end
end
end
end
else
end
end
return shipments
end

View Slide

require 'ostruct'
before { Object.send(:const_set, :Shipment, ShipmentDouble) }
after { Object.send(:remove_const, :Shipment) }
it “executes when consolidate is false" do
end
end
end
class ShipmentDouble < OpenStruct
def initialize(*)
super
self.line_items = []
end
end

View Slide

require 'ostruct'
end
it "executes when consolidate is true" do
assert ShipmentBuilder.new([order_line_item_double], true)
end
end
end
def initialize(*)
super
end
end

View Slide

# Running:

1) Error:
ShipmentBuilder#test_0002_executes when consolidate is true:
NameError: uninitialized constant ShipmentBuilder::Item
2 runs, assertions, 0 failures, 1 errors, 0 skips

View Slide

# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
shipments = []
unless consolidate
line_items = []
end
end
end
end
end
end
else
end
end
return shipments
end

View Slide

create_group_if_necessary_and_insert(
shipments, :consolidated, item.store_id, item.store_id, order_line_item
)
end

View Slide

shipments, :consolidated, order_line_item.store_id, order_line_item.store_id, order_line_item
)
end

View Slide

# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
shipments = []
unless consolidate
line_items = []
end
end
end
end
end
end
else
create_group_if_necessary_and_insert( shipments, :consolidated, order_line_item.store_id, order_line_item.store_id, order_line_item)
end
end
return shipments
end

View Slide

# Running:


View Slide

# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
end
... 
return shipments
end
...
end
end
"
> ShipmentBuilder.new([order_line_item1, order_line_item2])
=> [<#Shipment ...>, <#Shipment ...>]

View Slide

require 'ostruct'
end
end
end
end
def initialize(*)
super
end
end

View Slide

require 'ostruct'
it "returns a single in_stock shipment" do
shipments = ShipmentBuilder.new([order_line_item_double, order_line_item_double]).build_shipments
shipments.length.must_equal 1
shipments.first.shipment_type.must_equal :in_stock
end
end
end
end
def initialize(*)
super
end
end

View Slide

require 'ostruct'
end
it "returns a single consolidated shipment when consolidate is true" do
shipments = ShipmentBuilder.new([order_line_item_double, order_line_item_double], true).build_shipments
shipments.first.shipment_type.must_equal :consolidated
end
end
end
def initialize(*)
super
end
end

View Slide

# Running:

1) Error:
ShipmentBuilder#test_0001_returns a single in_stock shipment:
ArgumentError: wrong number of arguments (0 for 2)
shipment_builder.rb:16:in `build_shipments'
2) Error:
ShipmentBuilder#test_0002_returns a single consolidated shipment when consolidate is true:
ArgumentError: wrong number of arguments (0 for 2)
shipment_builder.rb:16:in `build_shipments'

View Slide

# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
end
... 
return shipments
end
...
end
end

View Slide

# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
attr_reader :order_line_items, :consolidate
end
... 
return shipments
end
...
end
end

View Slide

# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@order_line_items = order_line_items
@consolidate = consolidate
end
... 
return shipments
end
...
end
end

View Slide

# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
end
def build_shipments
... 
return shipments
end
...
end
end

View Slide

# Running:


View Slide

require 'ostruct'
end
shipments = ShipmentBuilder.new([order_line_item_double, order_line_item_double], true).build_shipments
shipments.first.shipment_type.must_equal :consolidated
end
end
end
def initialize(*)
super
end
end

View Slide

require 'ostruct'
order_line_items = [
order_line_item_double(ship_status_symbol: :in_stock),
order_line_item_double(ship_status_symbol: :drop_ship),
order_line_item_double(ship_status_symbol: :order_in),
]
shipments = ShipmentBuilder.new(order_line_items, true).build_shipments
end
end
it "returns multiple shipments when types and vendors differ" do
order_line_item_double(ship_status_symbol: :in_stock),
order_line_item_double(ship_status_symbol: :drop_ship, vendor_id: 9),
order_line_item_double(ship_status_symbol: :order_in),
]
shipments = ShipmentBuilder.new(order_line_items).build_shipments
shipments.count{|s| s.shipment_type == :drop_ship}.must_equal 2
end
it "consolidates drop ship items from the same vendor" do
]
end
it "consolidates in_stock to drop_ship if all in_stock items can be consolidated" do
order_line_item_double(ship_status_symbol: :in_stock, vendor_id: 9),
]
shipments.first.shipment_type.must_equal :drop_ship
end
it "does not consolidate in_stock to drop ship if any in_stock items are not consolidatable" do
]
shipments.find{|s| s.shipment_type == :drop_ship}.line_items.length.must_equal 1
end
end
def order_line_item_double(overrides = {})
line_item_params = { :item_id => 1, :store_id => 1, :vendor_id => 1, :drop_shippable? => true,
:ship_status_symbol => :in_stock }.merge(overrides)
OpenStruct.new(line_item_params)
end
def initialize(*)
super
end
end

View Slide

# Running:


View Slide

Katrina Owen
Therapeutic Refactoring

View Slide

end
def build_shipments
shipments = []
unless consolidate
line_items = []
end
end
end
end
end
end
else
end
end
return shipments
end
found = false
found = true
end
end
if found == false
end
end

View Slide

Strunk and White
2.17 Omit Needless Words
Vigorous writing is concise. A sentence
should contain no unnecessary words, a
paragraph no unnecessary sentences, for
the same reason that a drawing should
have no unnecessary lines and a machine
no unnecessary parts.

View Slide

end
def build_shipments
shipments = []
unless consolidate
line_items = []
end
end
end
end
end
end
else
end
end
return shipments
end
found = false
found = true
end
end
if found == false
end
end

View Slide

line_items = []
end

View Slide

line_items = []
:consolidatable => order_line_item.consolidatable)
end

View Slide

line_items = []
end
# Running:


View Slide

line_items = []
end

View Slide

def build_shipments
shipments = []
unless consolidate
line_items = []
:drop_shippable => order_line_item.drop_shippable
end
end
end
end
end
end
else
create_group_if_necessary_and_insert( shipments, :consolidated, order_line_item.store_id, order_line_item.store_id,
order_line_item)
end
end
return shipments
end

View Slide

def build_shipments
shipments = []
unless consolidate
line_items = order_line_items
end
end
end
end
end
else
order_line_item)
end
end
return shipments
end

View Slide

def build_shipments
shipments = []
unless consolidate
end
end
end
end
li.ship_status_symbol == :drop_ship ? li.vendor_id : li.store_id, li )
end
else
order_line_item)
end
end
return shipments
end

View Slide

# Running:


View Slide

end
def build_shipments
shipments = []
unless consolidate
end
end
end
end
end
else
order_line_item)
end
end
return shipments
end
...
end
end

View Slide

attr_reader :line_items, :consolidate
def initialize(line_items, consolidate = false)
@line_items = line_items
end
def build_shipments
shipments = []
unless consolidate
end
end
end
end
end
else
order_line_item)
end
end
return shipments
end
...
end
end

View Slide

end
def build_shipments
shipments = []
unless consolidate
end
end
end
end
end
else
order_line_item)
end
end
return shipments
end
...
end
end

View Slide

end
def build_shipments
shipments = []
unless consolidate
end
end
end
end
end
else
line_items.each do |line_item|
create_group_if_necessary_and_insert( shipments, :consolidated, line_item.store_id, line_item.store_id, line_item)
end
end
return shipments
end
...
end
end

View Slide

# Running:


View Slide

end
def build_shipments
shipments = []
unless consolidate
line_items_by_sym[ship_sym] = line_items.select{|li| li.ship_status == ship_sym}
end
end
line_items.each{|li| li.ship_status = :drop_ship if li.ship_status == :in_stock}
end
end
create_group_if_necessary_and_insert( shipments, li.ship_status, li.store_id,
li.ship_status == :drop_ship ? li.vendor_id : li.store_id, li )
end
else
end
end
return shipments
end
...
end
end

View Slide

Strunk and White
2.13 Make the paragraph the unit of composition
[A] subject requires division into topics,
each of which should be dealt with in a
paragraph. The object of treating each
topic in a paragraph by itself is, of course,
to aid the reader.

View Slide

end
def build_shipments
shipments = []
unless consolidate
end
end
end
end
end
else
end
end
return shipments
end
...
end
end

View Slide

end
def build_shipments
shipments = []
unless consolidate
end
end
end
end
end
else
end
end
return shipments
end

View Slide

end
def build_shipments
shipments = []
unless consolidate
end
end
end
end
end
else
end
end
return shipments
end
def build_consolidated_shipment
end
end

View Slide

end
def build_shipments
shipments = []
unless consolidate
end
end
end
end
end
else
build_consolidated_shipment
end
return shipments
end
end
end

View Slide

# Running:

1) Error:
ShipmentBuilder#test_0001_returns a single consolidated shipment when consolidate is true:
NameError: undefined local variable or method `shipments' for #
/Users/nmeans/Projects/krw_talk/shipment_builder.rb:54:in `block in build_consolidated_shipment'

View Slide

end
def build_shipments
shipments = []
unless consolidate
end
end
end
end
end
else
end
return shipments
end
end
end

View Slide

attr_reader :line_items, :consolidate, :shipments
@shipments = []
end
def build_shipments
unless consolidate
end
end
end
end
end
else
end
return shipments
end
end
end

View Slide

# Running:


View Slide

@shipments = []
end
def build_shipments
unless consolidate
end
end
end
end
end
else
end
return shipments
end
end
end

View Slide

@shipments = []
end
def build_shipments
unless consolidate
build_shipments_by_ship_status
else
end
return shipments
end
end
end
def build_shipments_by_ship_status
end
end
end
end
li.ship_status == :drop_ship ? li.vendor_id : li.store_id, li.line_item )
end
end

View Slide

# Running:


View Slide

@shipments = []
end
def build_shipments
unless consolidate
else
end
return shipments
end
end
end
end
end
end
end
end
end

View Slide

end
end
end
end
li.ship_status == :drop_ship ? li.vendor_id : li.store_id,
li.line_item )
end
end

View Slide

end
end
end
li.line_item )
end
end
def line_items_by_sym
end
line_items_by_sym
end

View Slide

# Running:


View Slide

end
end
end
li.line_item )
end
end
end
line_items_by_sym
end

View Slide

Strunk and White
2.16 Use deﬁnite, speciﬁc, concrete language
Prefer the specific to the general, the
definite to the vague, the concrete to the
abstract.

View Slide

end
end
end
li.line_item )
end
end
end
line_items_by_sym
end

View Slide

end
end
end
li.line_item )
end
end
end
line_items_by_sym
end
def line_items_by_ship_status(status)
line_items.select{|li| li.ship_status == status}
end

View Slide

end
end
end
li.line_item )
end
end
end

View Slide

if line_items_by_ship_status(:in_stock).length > 0 && line_items_by_sym[:drop_ship].length > 0
end
end
end
li.line_item )
end
end
end

View Slide

if in_stock_items.length > 0 && line_items_by_sym[:drop_ship].length > 0
end
end
end
li.line_item )
end
end
end

View Slide

end
end
end
li.line_item )
end
end
def in_stock_items
line_items_by_ship_status(:in_stock)
end
end

View Slide

in_stock_items.each do |li|
end
if in_stock_items.count{|li| !li.consolidatable} < 1
end
end
li.line_item )
end
end
def in_stock_items
end
end

View Slide

end
end
end
li.line_item )
end
end
def in_stock_items
end
def drop_ship_items
line_items_by_ship_status(:drop_ship)
end
end

View Slide

if in_stock_items.length > 0 && drop_ship_items.length > 0
matching_ds = drop_ship_items.count{|dsli| dsli.vendor_id == li.vendor_id} > 0
end
end
end
li.line_item )
end
end
def in_stock_items
end
def drop_ship_items
end
end

View Slide

# Running:


View Slide

end
end
end
li.line_item )
end
end
def in_stock_items
end
def drop_ship_items
end
end

View Slide

end
in_stock_items.each{|li| li.ship_status = :drop_ship}
end
end
li.line_item )
end
end
def in_stock_items
end
def drop_ship_items
end
end

View Slide

end
end
end
li.line_item )
end
end
def in_stock_items
end
def drop_ship_items
end
end

View Slide

end
end
li.line_item )
end
end
def consolidate_to_drop_ships?
end
in_stock_items.count{|li| !li.consolidatable} < 1
end
def in_stock_items
end
def drop_ship_items
end
end

View Slide

if consolidate_to_drop_ships?
end
end
li.line_item )
end
end
end
end
def in_stock_items
end
def drop_ship_items
end
end

View Slide

# Running:


View Slide

end
end
li.line_item )
end
end
end
end
def in_stock_items
end
def drop_ship_items
end
end

View Slide

end
end
li.line_item )
end
end
end
in_stock_items.all?(&:consolidatable)
end
def in_stock_items
end
def drop_ship_items
end
end

View Slide

end
end
li.line_item )
end
end
matching_ds = drop_ship_items.any?{|drop_ship_item| drop_ship_item.vendor_id == li.vendor_id}
end
end
def in_stock_items
end
def drop_ship_items
end
end

View Slide

end
end
li.line_item )
end
end
in_stock_items.each do |in_stock_item|
matching_ds = drop_ship_items.any?{|drop_ship_item| drop_ship_item.vendor_id == in_stock_item.vendor_id}
in_stock_item.consolidatable = (matching_ds && in_stock_item.drop_shippable)
end
end
def in_stock_items
end
def drop_ship_items
end
end

View Slide

end
end
li.line_item )
end
end
return false unless in_stock_items.all?(&:drop_shippable)
in_stock_item.consolidatable = (matching_ds && in_stock_item.drop_shippable)
end
end
def in_stock_items
end
def drop_ship_items
end
end

View Slide

end
end
li.line_item )
end
end
in_stock_item.consolidatable = (matching_ds)
end
end
def in_stock_items
end
def drop_ship_items
end
end

View Slide

end
end
li.line_item )
end
end
in_stock_item.consolidatable = drop_ship_items.any?{|drop_ship_item| drop_ship_item.vendor_id ==
in_stock_item.vendor_id}
end
end
def in_stock_items
end
def drop_ship_items
end
end

View Slide

end
end
li.line_item )
end
end
in_stock_items.map(&:vendor_id)
end
end
def in_stock_items
end
def drop_ship_items
end
end

View Slide

end
end
li.line_item )
end
end
in_stock_items.map(&:vendor_id) - drop_ship_items.map(&:vendor_id)
end
end
def in_stock_items
end
def drop_ship_items
end
end

View Slide

end
end
li.line_item )
end
end
(in_stock_items.map(&:vendor_id) - drop_ship_items.map(&:vendor_id)).empty?
end
end
def in_stock_items
end
def drop_ship_items
end
end

View Slide

end
end
li.line_item )
end
end
end
def in_stock_items
end
def drop_ship_items
end
end

View Slide

# Running:


View Slide

end
end
li.line_item )
end
end
end
def in_stock_items
end
def drop_ship_items
end
end

View Slide

end
li.line_item )
end
end
end
def in_stock_items
end
def drop_ship_items
end
end

View Slide

# Running:


View Slide

end
li.line_item )
end
end
end
def in_stock_items
end
def drop_ship_items
end
end

View Slide

end
li.line_item )
end
end
def consolidate_to_drop_ships
end
end
def in_stock_items
end
def drop_ship_items
end
end

View Slide

consolidate_to_drop_ships if consolidate_to_drop_ships?
li.line_item )
end
end
end
end
def in_stock_items
end
def drop_ship_items
end
end

View Slide

end
end
end
def in_stock_items
end
def drop_ship_items
end
end
def assign_items_to_shipments
li.line_item )
end
end

View Slide

assign_items_to_shipments
end
end
end
def in_stock_items
end
def drop_ship_items
end
end
li.line_item )
end
end

View Slide

# Running:


View Slide

# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@shipments = []
end
def build_shipments
unless consolidate
else
end
return shipments
end
end
end
end
end
end
def in_stock_items
end
def drop_ship_items
end
end
end
end
found = false
found = true
end
end
if found == false
end
end
end

View Slide

@shipments = []
end
def build_shipments
unless consolidate
else
end
return shipments
end
create_group_if_necessary_and_insert( shipments, :consolidated, line_item.store_id, line_item.store_id,
line_item)
end
end
end

View Slide

@shipments = []
end
def build_shipments
unless consolidate
else
end
return shipments
end
line_item.ship_status = :consolidated
end
end
end

View Slide

@shipments = []
end
def build_shipments
unless consolidate
else
end
return shipments
end
end
def consolidate_to_single_shipment
line_items.each { |li| li.ship_status = :consolidated }
end
end

View Slide

@shipments = []
end
def build_shipments
unless consolidate
else
end
return shipments
end
consolidate_to_single_shipment
end
end
end

View Slide

@shipments = []
end
def build_shipments
unless consolidate
else
end
return shipments
end
consolidate_to_single_shipment if consolidate
end
end
end

View Slide

attr_reader :line_items, :single_shipment, :shipments
def initialize(line_items, single_shipment = false)
@single_shipment = single_shipment
@shipments = []
end
def build_shipments
unless consolidate
else
end
return shipments
end
consolidate_to_single_shipment if single_shipment
end
end
end

View Slide

@shipments = []
end
def build_shipments
return shipments
end
end
end
end

View Slide

# Running:

1) Failure:
ShipmentBuilder#test_0006_does not consolidate in_stock to drop_ship if any in_stock items are not
consolidatable [shipment_builder_spec.rb:65]:
Expected: 1
Actual: 2
2) Failure:
ShipmentBuilder#test_0005_consolidates in_stock to drop_ship if all in_stock items can be consolidated
[shipment_builder_spec.rb:53]:
Expected: 1
Actual: 2

View Slide

@shipments = []
end
def build_shipments
return shipments
end
end
end
end

View Slide

# Running:


View Slide

@shipments = []
end
def build_shipments
return shipments
end
end
end
end

View Slide

@shipments = []
end
def build_shipments
return shipments
end
def optimize_consolidation
end
end
end
end

View Slide

@shipments = []
end
def build_shipments
optimize_consolidation
return shipments
end
end
end
end
end

View Slide

@shipments = []
end
def build_shipments
return shipments
end
end
end

View Slide

# Running:


View Slide

@shipments = []
end
def build_shipments
return shipments
end
end
end
end
end
def in_stock_items
end
def drop_ship_items
end
end
end
end
found = false
found = true
end
end
if found == false
end
end
end

View Slide

attr_reader :line_items, :single_shipment, :shipments, :shipment_list
@shipments = []
@shipment_list = ShipmentList.new(shipments)
end
def build_shipments
return shipments
end
end
end
end
end
def in_stock_items
end
def drop_ship_items
end
end
line_items.each{|li| shipment_list.assign_to_shipment(li)}
end
class ShipmentList
attr_accessor :shipments
def initialize(shipments)
@shipments = shipments
end
def assign_to_shipment(line_item)
shipment = find_or_create_shipment(line_item.ship_status, line_item.vendor_id)
shipment.line_items << line_item
end
def find_or_create_shipment(type, shipper_id)
find_shipment(type, shipper_id) || create_shipment(type, shipper_id)
end
def find_shipment(type, shipper_id)
shipments.find do |shipment|
shipment.shipment_type == type && (shipment.shipper_id == shipper_id || shipment.shipment_type != :drop_ship)
end
end
def create_shipment(type, shipper_id)
shipments << new_shipment = Shipment.new(shipment_type: type, shipper_id: shipper_id, store_id: 1)
new_shipment
end
end
end

View Slide

So, How’d We Do?
Original Refactored
3

View Slide

So, How’d We Do?
Original Refactored
Flay Score for Class
3

View Slide

So, How’d We Do?
Original Refactored
Flay Score for Class 36
3

View Slide

So, How’d We Do?
Original Refactored
Flay Score for Class 36 0
3

View Slide

So, How’d We Do?
Original Refactored
Flog Score for Class
3

View Slide

So, How’d We Do?
Original Refactored
Flog Score for Class 180.4
3

View Slide

So, How’d We Do?
Original Refactored
Flog Score for Class 180.4 77.2
3

View Slide

So, How’d We Do?
Original Refactored
Flog Score for #build_shipments
3

View Slide

So, How’d We Do?
Original Refactored
Flog Score for #build_shipments 120.7
3

View Slide

So, How’d We Do?
Original Refactored
Flog Score for #build_shipments 120.7 3.0
3

View Slide

shipments = []
unless consolidate
line_items = []
end
end
end
end
end
end
else
end
end
return shipments
end

View Slide

shipments = []
unless consolidate
line_items = []
end
end
end
end
end
end
else
end
end
return shipments
end
def build_shipments
return shipments
end
"

View Slide

Why did it work?
V

View Slide

5.5 Revise and Rewrite
O

View Slide

5.5 Revise and Rewrite
O
Red, Green, Refactor

View Slide

O

View Slide

O
Gang of Four
POODR
etc. ad infinitum

View Slide

5.19 Do not take shortcuts
at the cost of clarity
O

View Slide

O
Law of Demeter

View Slide

O
“Law” of Demeter

View Slide

2.13 Make the paragraph the
unit of composition
O

View Slide

2.13 Make the paragraph the
unit of composition
O
Single Responsibility Principle

View Slide

“Any fool can write
code a computer can
understand. Good
programmers write
code humans can
understand.”
– Martin Fowler
Refactoring: Improve the Design of Existing Code

View Slide

We stopped along the road for a bite to eat. The cowboy went off
to have a spare tire patched, and Eddie and I sat down in a kind
of homemade diner. I heard a great laugh, the greatest laugh in
the world, and here came this rawhide oldtimer Nebraska farmer
with a bunch of other boys into the
diner; you could hear his raspy cries
clear across the plains, across the whole
gray world of them that day. Everybody
else laughed with him. He didn’t have a
care in the world and had the hugest
regard for everybody. I said to myself,
Wham, listen to that man laugh. That’s
the West, here I am in the West.

View Slide

Uncle Charles is saying that though he can anticipate that the
Deans might be predisposed to weigh what he avers as
coming from his possible appearance as a kind of cheerleader
for E.T.A., he can assure the assembled
Deans that all this is true, and that the
Academy has presently in residence
no fewer than a third of the
continent’s top thirty junior, in age
brackets all across the board, and that
I here, who go by ‘Hal,’ usually, am
‘right up there among the very cream.’

View Slide

He was a long time going to sleep. After a while he turned and looked at the man. His face in the small light
streaked with black from the rain like some old world thespian. Can I ask you something? he said.
Yes. Of course.
Are we going to die?
Sometime. Not now.
And we’re still going south.
Yes.
So we’ll be warm.
Yes.
Okay.
Okay what?
Nothing. Just okay.
Go to sleep.
Okay.
I’m going to blow out the lamp. Is that okay?
Yes. Thats okay.
And then later in the darkness: Can I ask you something?
Yes. Of course you can.
What would you do if I died?
If you died I would want to die too.
So you could be with me?
Yes. So I could be with you.
Okay.

View Slide

“I felt uneasy posing as an
expert on rhetoric when the
truth is I write by ear,
always with difficulty and
seldom with any exact
notion of what is taking
place under the hood.”
– E. B. White
Introduction to “Will Strunk”, 1957

View Slide

Thanks!
V
Twitter: @nmeans
Blog: http://nickol.as/
Code: https://github.com/nmeans/code_like_a_writer

View Slide

How To Code Like A Writer

How To Code Like A Writer

Nickolas Means

More Decks by Nickolas Means

Other Decks in Programming

Featured

Transcript