Share
Embed
Rules, Laws, and Gentle Guidelines
A talk about programming rules, and about what happens when you try to take them too far.
Video: https://youtu.be/BDXQ4pcbEBA
Andrew Radev
September 23, 2016
Transcript
98 slides
-
Object-Oriented Programming: An Objective Sense of Style K. Lieberherr, I.
Holland, A. Riel (1988) -
Formal definition The Law of Demeter for functions requires that
a method m of an object O may only invoke the methods of the following kinds of objects: • O itself • m's parameters • Any objects created/instantiated within m • O's direct component objects • A global variable, accessible by O, in the scope of m -
public class Paperboy { payment = 2.00; // “I want
my two dollars!” Wallet theWallet = myCustomer.getWallet(); if (theWallet.getTotalMoney() > payment) { theWallet.subtractMoney(payment); } else { // come back later and get my money } } -
Problems “the paperboy is being exposed to more information than
he needs to be” -
public class Paperboy { payment = 2.00; // “I want
my two dollars!” paidAmount = myCustomer.getPayment(payment); if (paidAmount == payment) { // say thank you and give customer a receipt } else { // come back later and get my money } } -
public class Paperboy { payment = 2.00; // “I want
my two dollars!” paidAmount = myCustomer.getPayment(payment); if (paidAmount == payment) { // say thank you and give customer a receipt } else { // come back later and get my money } } -
public class Paperboy { payment = 2.00; // “I want
my two dollars!” paidAmount = myCustomer.getWalletSubtractAmount(payment); if (paidAmount == payment) { // say thank you and give customer a receipt } else { // come back later and get my money } } -
public class Paperboy { payment = 2.00; // “I want
my two dollars!” paidAmount = myCustomer.subtractAmountFromWallet(payment); if (paidAmount == payment) { // say thank you and give customer a receipt } else { // come back later and get my money } } -
The above method appears to obey the Law of Demeter
[ ]. However, on closer inspection … [ / … This clearly violates the spirit of the Law. 7 The Interface -
class Cart < AgileRecord::Base belongs_to :user delegate :address, to: :user,
prefix: true # => user_address end -
class Cart < AgileRecord::Base def shipping_address # ... end end
class AnonymousCart def initialize(session) # ... end def shipping_address # guess default country/city based on # geoip, products, etc. end end -
class User < AvocadoRecord::Base attribute :send_weekly_email_summary?, :boolean attribute :send_daily_email_summary?, :boolean
# … 10 more email-related attributes … end def send_a_bunch_of_emails(user) if user.send_weekly_email_summary? SendWeeklySummary.perform_later(user) elsif user.send_daily_email_summary? SendDailySummary.perform_later(user) end end -
class User < AvocadoRecord::Base def email_settings EmailSettings.new(...) end end def
send_a_bunch_of_emails(user) if user.email_settings.weekly_summary? SendWeeklySummary.perform_later(user) elsif user.email_settings.daily_summary? SendDailySummary.perform_later(user) end end -
def send_a_bunch_of_emails(recipient, email_settings) if email_settings.weekly_summary? SendWeeklySummary.perform_later(recipient) elsif email_settings.daily_summary? SendDailySummary.perform_later(recipient) end
end send_a_bunch_of_emails(user, user.email_settings) send_a_bunch_of_emails( organization.member, organization.email_settings) -
def send_a_bunch_of_emails(user) email_settings = user.email_settings if email_settings.weekly_summary? SendWeeklySummary.perform_later(user) elsif email_settings.daily_summary?
SendDailySummary.perform_later(user) end end -
Law of Demeter 1) Naming 2) Method parameters 3) Not
really a “law” -
Writing programs which follow the the Law of Demeter [
/ … increases the number of methods. [ / … the abstraction may be less comprehensible, and implementation and maintenance are more difficult. There might also be an increase in the number of arguments passed to some methods. 6 The Trade-off -
class UpdateComment def initialize(comment) @comment = comment end def call
Comment.transaction do @comment.save! update_article_counter update_some_other_thing end end private def update_article_counter @comment.article.update_comment_count end def update_some_other_thing # ... end end -
class DestroyComment def initialize(comment) @comment = comment end def call
Comment.transaction do @comment.destroy update_article_counter update_some_other_thing end end private def update_article_counter @comment.article.update_comment_count end def update_some_other_thing # ... end end -
class CommentLifecycle def initialize(comment) @comment = comment end def update
Comment.transaction do @comment.save! update_article_counter update_some_other_thing end end def destroy Comment.transaction do @comment.destroy update_article_counter update_some_other_thing end end private # ... end -
class CommentLifecycle def initialize(comment) @comment = comment end def not_very_srp?
Comment.transaction do @comment.save! update_article_counter update_some_other_thing end end def destroy Comment.transaction do @comment.destroy update_article_counter update_some_other_thing end end private # ... end -
class CommentLifecycle def initialize(comment) @comment = comment end def not_very_srp?
Comment.transaction do @comment.save! update_article_counter update_some_other_thing end end def not_very_srp! Comment.transaction do @comment.destroy update_article_counter update_some_other_thing end end private # ... end -
Robert C. Martin Separation of Concerns, Coupling, Cohesion “In the
late 1990s I tried to consolidate these notions into a principle, which I called: The Single Responsibility Principle.” “[It] states that each software module should have one and only one reason to change.” -
“it should do one thing” “it does too much” “it
should have one reason to change” -
def fill_in_login_details(params) fill_in 'email', with: params[:email] fill_in 'password', with: params[:password]
end def fill_in_address(params) fill_in 'city', with: params[:city] fill_in 'country', with: params[:country] # ... end def fill_in_banking_info(params) fill_in 'iban', with: params[:iban] fill_in 'bic', with: params[:bic] click_button 'Submit' end -
def fill_in_banking_info(params) fill_in 'iban', with: params[:iban] fill_in 'bic', with: params[:bic]
click_button 'Submit' end -
def fill_in_banking_info(params) fill_in 'iban', with: params[:iban] fill_in 'bic', with: params[:bic]
end def submit_form click_button 'Submit' end -
def fill_in_banking_info(params) fill_in 'iban', with: params[:iban] fill_in 'bic', with: params[:bic]
click_button 'Submit' end -
def fill_in_banking_info(params) fill_in 'email', with: params[:email] fill_in 'password', with: params[:password]
fill_in 'city', with: params[:city] fill_in 'country', with: params[:country] # ... fill_in 'iban', with: params[:iban] fill_in 'bic', with: params[:bic] click_button 'Submit' end -
def fill_in_banking_info(params) visit '/register' fill_in 'email', with: params[:email] fill_in 'password',
with: params[:password] fill_in 'city', with: params[:city] fill_in 'country', with: params[:country] # ... fill_in 'iban', with: params[:iban] fill_in 'bic', with: params[:bic] click_button 'Submit' end -
def register_user(params) visit '/register' fill_in 'email', with: params[:email] fill_in 'password',
with: params[:password] fill_in 'city', with: params[:city] fill_in 'country', with: params[:country] # ... fill_in 'iban', with: params[:iban] fill_in 'bic', with: params[:bic] click_button 'Submit' end -
scenario "registration form creates a user" do register_user(user_params) expect_user_to_be_registered end
scenario "registration form sends an email" do register_user(user_params) expect_an_email_to_have_been_sent end scenario "invalid registration form" do register_user(invalid_user_params) expect_errors_in_the_form end -
def fill_in_banking_info(params) fill_in 'iban', with: params[:iban] fill_in 'bic', with: params[:bic]
click_button 'Submit' end -
scenario "shopping cart" do # ... fill_in_address(params) next_step # ...
fill_in_banking_info(params) next_step # ... end -
scenario "registration form" do visit '/register' fill_in_login_details(params) fill_in_address(params) fill_in_banking_info(params) submit_form
expect_user_to_be_registered end scenario "registration form" do visit '/register' fill_in_login_details(invalid_params) fill_in_address(invalid_params) fill_in_banking_info(invalid_params) submit_form expect_errors_in_form end -
def fill_in_banking_info(params) fill_in 'iban', with: params[:iban] fill_in 'bic', with: params[:bic]
click_button 'Submit' end -
Principles, Patterns, and Practices public interface Modem { public void
Dial(string pno); public void Hangup(); public void Send(char c); public char Recv(); } -
Responsibilities “Should these two responsibilities be separated? That depends upon
how the application is changing.” “[…] If, on the other hand, the application is not changing in ways that cause the the two responsibilities to change at different times, then there is no need to separate them.” “An axis of change is only an axis of change if the changes actually occur. It is not wise to apply the SRP, or any other principle for that matter, if there is no symptom.” – Robert C. Martin -
SRP 1) Naming 2) Redrawing the borders 3) “Reasons to
change” is an estimation -
<div class="product-wrapper"> <img class="product-image" src="<%= product.image_url %>"> <div class="product-info"> <h2
class="product-title"><%= product.title %></h2> <p class="product-description"> <%= product.description %> </p> </div> </div> -
-<div class="product-wrapper"> - <img class="product-image" - src="<%= product.image_url %>"> -
- <div class="product-info"> - <h2 class="product-title"><%= product.title %></h2> - - <p class="product-description"> - <%= product.description %> - </p> - </div> -</div> -
<% @products.each do |p| %> - <%= render 'product_card', product:
p %> + <%= SomeClass.some_method(p) %> <% end %> -
#define HTML \ "<div class=\"product-wrapper\"> \ <img class=\"product-image\" src=\"%s\"> \
<div class=\"product-info\"> \ <h2 class=\"product-title\">%s</h2> \ <p class=\"product-description\"> \ %s \ </p> \ </div> \ </div>" char data[1000]; sprintf(data, HTML, ...); -
??? • It’s fast! • Coded properly • “Use the
best tool for the job” • We’re not really going to touch it again • It’s already done -
“You can't fit a complex, context-dependent programming principle into a
tweet-length sentence.” – Radev’s First Law -
“Don’t force-push stuff to master at 3 in the morning”
– Radev’s Second Law