Мой сосед Легаси

Федоров Сергей, Evil Martians

Проект Amplifr

Музыкальная индустрия
4 соцсети
Публикации
Простая аналитика

Проект Amplifr

100% SMM
12 соцсетей
Публикации
Аналитика
Рекомендации
Рассылки

Проект Amplifr

Эволюция VS Революция

О чем доклад?

О простых и дешевых приемах, которые позволят вам и вашей команде эффективно работать с легаси кодом

Форматирование

Форматирование

Никому не нравится читать плохо отформатированный код

Форматирование

def write_stats(publication, total, data = {})
  activity = count_total_from_publication_data(publication)
  update_activity(publication, activity)
  total = activity.values.sum
  if override_stat_date?(publication, 'created_at')
    data[:date] = Time.parse(publication.data['created_at'])
  end
  unless dont_write_stats
    if plays_total = publication.data.try(:[], 'playback_count').to_i
      stat = \
        Stat.record_total(
          "soundcloud_plays",
          { project: publication.project, resource: publication }.merge(data),
          plays_total
        )
    end
  end
  super
end

Форматирование

def write_stats(publication, total, data = {})
  activity = count_total_from_publication_data(publication)
  total = activity.values.sum

  update_activity(publication, activity)

  if override_stat_date?(publication, 'created_at')
    data[:date] = Time.parse(publication.data['created_at'])
  end

  unless dont_write_stats
    if plays_total = publication.data.try(:[], 'playback_count').to_i
      stat = \
        Stat.record_total(
          "soundcloud_plays",
          { project: publication.project, resource: publication }.merge(data),
          plays_total
        )
    end
  end

  super
end

Форматирование

def write_stats(publication, total, data = {})
  activity = count_total_from_publication_data(publication)
  total = activity.values.sum

  update_activity(publication, activity)

  if override_stat_date?(publication, 'created_at')
    data[:date] = Time.parse(publication.data['created_at'])
  end

  unless dont_write_stats
    if plays_total = publication.data.try(:[], 'playback_count').to_i
      stat = \
        Stat.record_total(
          "soundcloud_plays",
          { project: publication.project, resource: publication }.merge(data),
          plays_total
        )
    end
  end

  super
end

Форматирование

def write_stats(publication, total, data = {})
  activity = count_total_from_publication_data(publication)
  total = activity.values.sum

  update_activity(publication, activity)

  if override_stat_date?(publication, 'created_at')
    data[:date] = Time.parse(publication.data['created_at'])
  end

  unless dont_write_stats
    if plays_total = publication.data.try(:[], 'playback_count').to_i
      stat = \
        Stat.record_total(
          "soundcloud_plays",
          { project: publication.project, resource: publication }.merge(data),
          plays_total
        )
    end
  end

  super
end

Форматирование

def write_stats(publication, total, data = {})
  activity = count_total_from_publication_data(publication)
  total = activity.values.sum

  update_activity(publication, activity)

  if override_stat_date?(publication, 'created_at')
    data[:date] = Time.parse(publication.data['created_at'])
  end

  unless dont_write_stats
    if plays_total = publication.data.try(:[], 'playback_count').to_i
      stat = \
        Stat.record_total(
          "soundcloud_plays",
          { project: publication.project, resource: publication }.merge(data),
          plays_total
        )
    end
  end

  super
end

Форматирование

def write_stats(publication, total, data = {})
  activity = count_total_from_publication_data(publication)
  total = activity.values.sum

  update_activity(publication, activity)

  if override_stat_date?(publication, 'created_at')
    data[:date] = Time.parse(publication.data['created_at'])
  end

  unless dont_write_stats
    if plays_total = publication.data.try(:[], 'playback_count').to_i
      stat = \
        Stat.record_total(
          "soundcloud_plays",
          { project: publication.project, resource: publication }.merge(data),
          plays_total
        )
    end
  end

  super
end

Форматирование

def write_stats(publication, total, data = {})
  activity = count_total_from_publication_data(publication)
  total = activity.values.sum

  update_activity(publication, activity)

  if override_stat_date?(publication, 'created_at')
    data[:date] = Time.parse(publication.data['created_at'])
  end

  unless dont_write_stats
    if plays_total = publication.data.try(:[], 'playback_count').to_i
      stat = \
        Stat.record_total(
          "soundcloud_plays",
          { project: publication.project, resource: publication }.merge(data),
          plays_total
        )
    end
  end

  super
end

Форматирование

Современные IDE поддерживают функцию автоматического форматирования кода

Форматирование

Концепции, тесно связанные друг с другом, должны находиться поблизости друг от друга по вертикали

Форматирование

Взаимозависимые функции должны размещаться в нисходящем порядке

Форматирование

Единый стиль форматирования уменьшает вероятность ошибки

Источник: Robert C. Martin: Clean Code, A Handbook of Agile Software Craftsmanship

Неправильные имена

Неправильные имена

Самое трудное в программировании — это придумывать имена

Неправильные имена

Комментарии могут обманывать так же, как и неверные имена

Неправильные имена

Плохое имя — как магнит для лишней функциональности

Неправильные имена

def access_token
  # We store only refresh token and for each request
  # just fetching new access token
  api = Odnoklassniki::Client.new(
    access_token: @account.token,
    client_id: Settings.odnoklassniki.app_id,
    client_secret: Settings.odnoklassniki.app_secret,
  )
  new_token = api.refresh_token!
  raise Social::API::Unauthorized if new_token.blank?
  api
end

Неправильные имена

def access_token
  # We store only refresh token and for each request
  # just fetching new access token
  api = Odnoklassniki::Client.new(
    access_token: @account.token,
    client_id: Settings.odnoklassniki.app_id,
    client_secret: Settings.odnoklassniki.app_secret,
  )
  new_token = api.refresh_token!
  raise Social::API::Unauthorized if new_token.blank?
  api
end

Неправильные имена

def access_token
  # We store only refresh token and for each request
  # just fetching new access token
  api = Odnoklassniki::Client.new(
    access_token: @account.token,
    client_id: Settings.odnoklassniki.app_id,
    client_secret: Settings.odnoklassniki.app_secret,
  )
  new_token = api.refresh_token!
  if new_token.blank?
    @account.disable!
    raise Social::API::Unauthorized
  end
  api
end

Неправильные имена

def rest_client_with_refreshed_token
  client = Odnoklassniki::Client.new(
    access_token: @account.token,
    client_id: Settings.odnoklassniki.app_id,
    client_secret: Settings.odnoklassniki.app_secret,
  )

  refreshed_token = client.refresh_token!
  raise Social::API::Unauthorized if refreshed_token.blank?

  client
end

Дублирование и DRY

Дублирование и DRY

Все ли дублирование стоит устранять?

Дублирование и DRY

module Social
  module API
    class Vkontakte
      def initialize(access_token)
        @access_token = access_token
      end

      # ...
    end
  end
end

module Social
  module API
    class Twitter
      def initialize(access_token)
        @access_token = access_token
      end

      # ...
    end
  end
end

Дублирование и DRY

module Social
  module API
    class Base
      def initialize(access_token)
        @access_token = access_token
      end
    end
  end
end

module Social
  module API
    class Vkontakte < Base
      # ...
    end
  end
end

Дублирование и DRY

module Social
  module API
    class Base
      def initialize(access_token)
        @access_token = access_token
      end

      def rescue_api_errors(error, args)
        # ..
      end

      def send_notification
        # ...
      end
    end
  end
end

Дублирование и DRY

Понятность кода не должна быть ценой устранения дублирования

Дублирование и DRY

С дублированием кода вполне можно жить

Дублирование и DRY

Неверные абстракции устранять сложнее, чем дублирование

Дублирование и DRY

Устраняя дублирование руководствуйтесь примерами использования кода

Интерфейсы вместо методов

Интерфейсы вместо методов

Лучший код — код которого нет

Интерфейсы вместо методов

Копирование или расширение существующих методов — ложный reuse

Интерфейсы вместо методов

def fetch_account_publications(opts = {})
  all = api_fetch
  posts = all['feeds']
  entities = all['entities'].try(:[], 'media_topics') || []

  return unless posts.present? || entities.present?

  posts.each do |story|
    publication = find_publication(story)
    likes = entities.find { |it| it['ref'] == story['target_refs'].try(:[], 0) }
    likes = likes['like_summary']['count'] if likes
    likes ||= 0
    comments = story['discussion_summary'].try(:[], 'comments_count').to_i
    shares = 0
    update_activity(publication, {like: likes, share: shares, comment: comments})
    total = comments + likes
    write_stats(publication, total) if publication.is_a?(Social::ExternalPublication)
  end
end

Интерфейсы вместо методов

def fetch_account_publications(opts = {})
  all = api_fetch
  posts = all['feeds']
  entities = all['entities'].try(:[], 'media_topics') || []

  return unless posts.present? || entities.present?

  posts.each do |story|
    publication = find_publication(story)
    likes = entities.find { |it| it['ref'] == story['target_refs'].try(:[], 0) }
    likes = likes['like_summary']['count'] if likes
    likes ||= 0
    comments = story['discussion_summary'].try(:[], 'comments_count').to_i
    shares = 0
    update_activity(publication, {like: likes, share: shares, comment: comments})
    total = comments + likes
    write_stats(publication, total) if publication.is_a?(Social::ExternalPublication)
  end
end

Интерфейсы вместо методов

def fetch_account_publications(opts = {})
  all = api_fetch
  posts = all['feeds']
  entities = all['entities'].try(:[], 'media_topics') || []

  return unless posts.present? || entities.present?

  posts.each do |story|
    publication = find_publication(story)
    likes = entities.find { |it| it['ref'] == story['target_refs'].try(:[], 0) }
    likes = likes['like_summary']['count'] if likes
    likes ||= 0
    comments = story['discussion_summary'].try(:[], 'comments_count').to_i
    shares = 0
    update_activity(publication, {like: likes, share: shares, comment: comments})
    total = comments + likes
    write_stats(publication, total) if publication.is_a?(Social::ExternalPublication)
  end
end

Интерфейсы вместо методов

def get_publication_activity(story)
  publication = find_publication(story)
  likes = entities.find { |it| it['ref'] == story['target_refs'].try(:[], 0) }
  likes = likes['like_summary']['count'] if likes
  likes ||= 0
  comments = story['discussion_summary'].try(:[], 'comments_count').to_i
  shares = 0

  {likes: likes, comments: comments, shares: shares}
end

Интерфейсы вместо методов

class PublicationStatistics
  def likes
    # ...
  end

  def comments
    # ...
  end

  def shares
    # ...
  end
end

Интерфейсы вместо методов

Производите анализ имеющегося кода

Тесты

Тесты

Код без тестов — легаси код

Источник: Michael Feathers: Working Effectively with Legacy Code

Тесты

Как бы небыл понятен код — не факт, что ты ничего не сломал

Тесты

Критичные участки кода должны быть покрыты тестами

Рефакторинг

Рефакторинг

Довольно дорогой процесс для бизнеса

Рефакторинг

Очень сложно уложиться в отведенное время

Рефакторинг

Параллельная разработка делает рефакторинг в разы сложнее

Заключение

Правило бойскаута — оставь место стоянки чище, чем оно было до твоего прихода

Заключение

Теория разбитых окон — устраняя малозначительные недочеты, вы не допускаете более крупных

Заключение

Разработка через рефакторинг

Вопросы

Почта: strech@evl.ms
Твиттер: @oni_strech
Слайды: strech.github.io/my-neighbor-legacy