How to write good code.

Transcript

Radoslav Stankov VarnaConf 11/08/2012 How to write good code

How am I? @rstankov http://rstankov.com http://github.com/rstankov

None

<ul class="accordion-menu"> <li class="item"> <a href="#" class="section-1">Section 1</a> <ul id="section-1">

$('.accordion-menu .item a').each(function() { $(this).bind('click', function() { var title =

this.className; var theul = $('#' + title); if (theul.length > 0) { if (theul.css('display') == 'none') { theul.slideDown(); var theli = dropa.parent('li'); theli.animate({'padding-bottom': '20px'}); } else { theul.slideUp(); var theli = dropa.parent('li'); theli.animate({'padding-bottom': '20px'}); } } else { } }); });

$('.accordion-menu .item a').each(function() { $(this).bind('click', function() { var title =

this.className; var theul = $('#' + title); if (theul.length > 0) { if (theul.css('display') == 'none') { theul.slideDown(); var theli = dropa.parent('li'); theli.animate({'padding-bottom': '20px'}); } else { theul.slideUp(); var theli = dropa.parent('li'); theli.animate({'padding-bottom': '20px'}); } } else { } }); });

$('.accordion-menu .item a').each(function() { $(this).bind('click', function() { var title =

this.className; var theul = $('#' + title); if (theul.length > 0) { if (theul.css('display') == 'none') { theul.slideDown(); var theli = dropa.parent('li'); theli.animate({'padding-bottom': '20px'}); } else { theul.slideUp(); var theli = dropa.parent('li'); theli.animate({'padding-bottom': '20px'}); } } }); });

$('.accordion-menu .item a').each(function() { $(this).bind('click', function() { var title =

this.className; var theul = $('#' + title); if (theul.length > 0) { if (theul.css('display') == 'none') { theul.slideDown(); var theli = dropa.parent('li'); theli.animate({'padding-bottom': '20px'}); } else { theul.slideUp(); var theli = dropa.parent('li'); theli.animate({'padding-bottom': '20px'}); } } }); });

$('.accordion-menu .item a').click(function() { var title = this.className; var theul

= $('#' + title); if (theul.length > 0) { if (theul.css('display') == 'none') { theul.slideDown(); var theli = dropa.parent('li'); theli.animate({'padding-bottom': '20px'}); } else { theul.slideUp(); var theli = dropa.parent('li'); theli.animate({'padding-bottom': '20px'}); } } });

$('.accordion-menu .item a').click(function() { var title = this.className; var theul

<ul class="accordion-menu"> <li class="item"> <a href="#" class="section-1">Section 1</a> <ul id="section-1">

$('.accordion-menu .item a').click(function() { var title = this.className; var theul

$('.accordion-menu .item a').click(function() { var theul = $(this).next('ul'); if (theul.length

> 0) { if (theul.css('display') == 'none') { theul.slideDown(); var theli = dropa.parent('li'); theli.animate({'padding-bottom': '20px'}); } else { theul.slideUp(); var theli = dropa.parent('li'); theli.animate({'padding-bottom': '20px'}); } } });

$('.accordion-menu .item a').click(function() { var theul = $(this).next('ul'); if (theul.length

> 0) { if (theul.css('display') == 'none') { theul.slideDown() } else { theul.slideUp() } var theli = dropa.parent('li') theli.animate({'padding-bottom': '20px'}); } });

$('.accordion-menu .item a').click(function() { var theul = $(this).next('ul'); if (theul.length

> 0) { if (theul.css('display') == 'none') { theul.slideDown() } else { theul.slideUp() } var theli = dropa.parent('li') theli.animate({'padding-bottom': '20px'}); } });

$('.accordion-menu .item a').click(function() { var theul = $(this).next('ul') if (theul.length

> 0) { if (theul.css('display') == 'none') { theul.slideDown() } else { theul.slideUp() } } });

$('.accordion-menu .item a').click(function() { var theul = $(this).next('ul') if (theul.length

> 0) { if (theul.css('display') == 'none') { theul.slideDown() } else { theul.slideUp() } } });

$('.accordion-menu .item a').click(function() { var theul = $(this).next('ul') if (theul.length

> 0) { if (theul.is(':visible')) { theul.slideDown() } else { theul.slideUp() } } });

$('.accordion-menu .item a').click(function() { var theul = $(this).next('ul') if (theul.length

> 0) { theul.slideToggle(); } });

$('.accordion-menu .item a').click(function() { var theul = $(this).next('ul') if (theul.length

> 0) { theul.slideToggle(); } });

$('.accordion-menu .item a').click(function() { $(this).next('ul').slideToggle(); });

$('.accordion-menu .item a').live('click', function() { $(this).next('ul').slideToggle(); });

$('.accordion-menu .item a').click(function() { $(this).next('ul').slideToggle(); });

$(document).on('click', '.accordion-menu .item a', function() { $(this).next('ul').slideToggle(); });

$(document).on('click', '.js-accordion-item a', function() { $(this).next('js-accordion-section').slideToggle(); });

$(document).on('click', '[data-accordion="item"]', function() { $(this).next('[data-accordion="section"]').slideToggle(); });

Know your tools

None

class NewsestFriendImages def initialize(user) @user = user end def images(page)

friends = Friendship.where(user_id: @user.id).pluck('friend_id') images = Image.where(user_id: friends, visible_to_friends: true) images = images.where('minimum_age >= ?', @user.age) images = images.order('created_at') images = images.limit(PER_PAGE).offset(page.to_i.abs * PER_PAGE) images.map { |image| ImageDecorator.new(image) } end end NewsestFriendImages.new(current_user).images(params[:page])

class NewsestFriendImages def initialize(user) @user = user end def images(page)

friends = Friendship.where(user_id: @user.id).pluck('friend_id') images = Image.where(user_id: friends, visible_to_friends: true) images = images.where('minimum_age >= ?', @user.age) images = images.order('created_at') images = images.limit(PER_PAGE).offset(page.to_i.abs * PER_PAGE) images.map { |image| ImageDecorator.new(image) } end end NewsestFriendImages.new(current_user).images(params[:page]) Copy & Paste

class MostLikedFriendImages def initialize(user) @user = user end def images(page)

friends = Friendship.where(user_id: @user.id).pluck('friend_id') images = Image.where(user_id: friends, visible_to_friends: true) images = images.where('minimum_age >= ?', @user.age) images = images.order('votes_count') images = images.limit(PER_PAGE).offset(page.to_i.abs * PER_PAGE) images.map { |image| ImageDecorator.new(image) } end end MostLikedFriendImages.new(current_user).images(params[:page])

class MostLikedFriendImages def initialize(user) @user = user end def images(page)

DRY don’t repeat yourself

Every piece of knowledge must have a single, unambiguous, authoritative

representation within a system.

class NewsestFriendImages def initialize(user) @user = user end def images(page)

class FriendImages def initialize(user) @user = user end def images(sorting,

page) friends = Friendship.where(user_id: @user.id).pluck('friend_id') images = Image.where(user_id: friends, visible_to_friends: true) images = images.where('minimum_age >= ?', @user.age) images = images.order(sorting) images = images.limit(PER_PAGE).offset(page.to_i.abs * PER_PAGE) images.map { |image| ImageDecorator.new(image) } end end FriendImages.new(current_user).images('created_at', params[:page]) FriendImages.new(current_user).images('likes_count', params[:page])

class FriendImages def initialize(user) @user = user end def images(sorting,

The bad side of DRY

http://bit.ly/O5jSLV

API_GET = '/api/:source_type/:id' API_GET_DETAILS = '/api/:source_type/:id/details' API_GET_VERSION = '/api/:source_type/:id/on/:year/:date/:month' API_GET_SUBTYPE

= '/api/:source_type/:id/sub/:subtype/:subtype_id'

API = 'api' SOURCE_TYPE = '/:source_type' SOURCE_ID = '/:id' SOURCE_DATE

= '/:year/:month/:day' SUBTYPE = '/:subtype' SUBID = '/:subtype_id' API_GET = API + SOURCE_TYPE + SOURCE_ID API_GET_DETAILS = API + SOURCE_TYPE + SOURCE_ID + '/details' API_GET_VERSION = API + SOURCE_TYPE + SOURCE_ID + '/on' + SOURCE_DATE API_GET_SUBTYPE = API + SOURCE_TYPE + SOURCE_ID + '/sub' + SUBTYPE + SUBID

def api_path(rest = '') BASE_PATH + rest end def token(string)

SEPARATOR + string end SEPARATOR = '/' API = 'api' SOURCE_TYPE = ':source_type' SOURCE_ID = ':id' YEAR = ':year' MONTH = ':month' DAY = ':day' SUBTYPE = ':subtype' SUBID = ':subtype_id' DETAILS = 'details' ON = 'on' SUB = 'sub' BASE_PATH = token(API) + token(SOURCE_TYPE) + token(SOURCE_ID) DATE_PART = token(YEAR) + token(MONTH) + token(DAY) SUB_PART = token(SUBTYPE) + token(SUBID) API_GET = api_path() API_GET_DETAILS = api_path(token(DETAILS)) API_GET_VERSION = api_path(token(ON) + token(DATE_PART)) API_GET_SUBTYPE = api_path(token(SUB) + token(SUB_PART))

None

class FriendImages def initialize(user) @user = user end def images(sorting,

page) friends = Friendship.where(user_id: @user.id).pluck('friend_id') images = Image.where(user_id: friends, visible_to_friends: true) images = images.where('minimum_age >= ?', @user.age) images = images.order(sorting) images = images.limit(PER_PAGE).offset(page.to_i.abs * PER_PAGE) images.map { |image| ImageDecorator.new(image) } end end

SLAP Single Level of Abstraction Principle

Keep all lines of code in a method at the

same level of abstraction.

class FriendImages def initialize(user) @user = user end def images(sorting,

page) friends = Friendship.where(user_id: @user.id).pluck('friend_id') images = Image.where(user_id: friends, visible_to_friends: true) images = images.where('minimum_age >= ?', @user.age) images = images.order(sorting) images = images.limit(PER_PAGE).offset(page.to_i.abs * PER_PAGE) images.map { |image| ImageDecorator.new(image) } end end

def images(sorting, page) friends = Friendship.where(user_id: @user.id).pluck('friend_id') images = Image.where(user_id:

friends, visible_to_friends: true) images = images.where('minimum_age >= ?', @user.age) images = images.order(sorting) images = images.limit(PER_PAGE).offset(page.to_i.abs * PER_PAGE) images.map { |image| ImageDecorator.new(image) } end

def images(sorting, page) images = find_images_of(friends) images = apply_age_restritions_to(images) images

= order_by(images, sorting) images = paginate(images, page) decorate(images) end

def friends Friendship.where(user_id: @user.id).pluck('friend_id') end def find_images_of(friends) Image.where(user_id: friends, visible_to_friends:

true) end def apply_age_restritions_to(images) images.where('minimum_age >= ?', @user.age) end def order_by(sorting) images.order(sorting) end def paginate(page) images.limit(PER_PAGE).offset(page.to_i.abs * PER_PAGE) end def decorate(images) images.map { |image| ImageDecorator.new(image) } end

Law of Demeter

• You can play with yourself. • You can play

with your own toys (but you can't take them apart), • You can play with toys that were given to you. • And you can play with toys you've made yourself.

• Your method can call other methods in its class

directly • Your method can call methods on its own fields directly (but not on the fields' fields) • When your method takes parameters, your method can call methods on those parameters directly. • When your method creates local objects, that method can call methods on the local objects.

def friends Friendship.where(user_id: @user.id).pluck('friend_id') end def find_images_of(friends) Image.where(user_id: friends, visible_to_friends:

true) end def apply_age_restritions_to(images) images.where('minimum_age >= ?', @user.age) end def order_by(sorting) images.order(sorting) end def paginate(page) images.limit(PER_PAGE).offset(page.to_i.abs * PER_PAGE) end def decorate(images) images.map { |image| ImageDecorator.new(image) } end

def friends Friendship.friend_ids_of(@user) end def find_images_of(friends) Image.of(friends) end def apply_age_restritions_to(images)

images.appropriate_for(@user.age) end def order_by(sorting) images.order(sorting) end def paginate(page) images.paginate(page) end def decorate(images) images.map { |image| ImageDecorator.new(image) } end

def images(sorting, page) images = find_images_of(friends) images = apply_age_restritions_to(images) images

= order_by(images, sorting) images = paginate(images, page) decorate(images) end

def images(sorting, page) decorate Image .of(friends) .appropriate_for(@user.age) .order(sorting) .paginate(page) end

def images(page) friends = Friendship.where(user_id: @user.id).pluck('friend_id') images = Image.where(user_id: friends,

visible_to_friends: true) images = images.where('minimum_age >= ?', @user.age) images = images.order('created_at') images = images.limit(PER_PAGE).offset(page.to_i.abs * PER_PAGE) images.map { |image| ImageDecorator.new(image) } end

def images(sorting, page) decorate Image .of(friends) .appropriate_for(@user.age) .order(sorting) .paginate(page) end

Code smells

• Duplicated Code • Long Method • Large Class •

Long Parameter List • Shotgun Surgery • Feature Envy • Data Clumps • Case Statements • Parallel Inheritance Hierarchies • Speculative Generality • Temporary Field • Message Chains • Middle Man • Alternative Classes with Different Interfaces • Refused Bequest • Comments • Repetitive Boilerplate • и т.н....

How to write good code.

How to write good code.

More Decks by Radoslav Stankov

Other Decks in Technology

Featured

Transcript