たとえばこのようなコードがある．Twitter でいいねした画像付きツイートを保存して，その URL を Slack に投稿して，サムネイル画像を生成して，必要な情報を DB に書き込むという処理をやっている．

client.on_event(:favorite) do |event|
  source = event[:source]
  target_object = event[:target_object]
  if source[:screen_name] == YOUR_SCREEN_NAME && !target_object[:user][:protected] && target_object[:extended_entities]
    id_str = target_object[:id_str]
    screen_name = target_object[:user][:screen_name]
    url = create_twitter_url(id_str, screen_name)
    download_urls = []
    target_object[:extended_entities][:media].size.times do |i|
      media = target_object[:extended_entities][:media][i]
      download_url = media[:media_url_https]
      download_urls << download_url
    end
    text = url
    puts text
    options[:text] = text
    Slack.chat_postMessage(options) if filtering(target_object)
    download_urls.each do |download_url|
      download_filepath = File.join(IMAGE_DOWNLOAD_DIR, File::basename(download_url))
      thumbnail_filepath = File.join(THUMBNAIL_DIR, File::basename(download_url))
      begin
        body = open(download_url + ':orig', &:read)
        download_url = download_url + ':orig'
      rescue OpenURI::HTTPError
        begin
          body = open(download_url, &:read)
        rescue OpenURI::HTTPError
          raise '画像のダウンロードに失敗しました'
        end
      end
      puts download_url
      File.binwrite(download_filepath, body)
      Magick::Image.from_blob(body).shift.resize_to_fit(128).write thumbnail_filepath
      db.transaction
      begin
        db.execute INSERT_SQL, download_filepath, screen_name, id_str, Time.now.to_f, target_object[:text]
        db.commit
      rescue
        db.rollback
      end
    end
  end
end

私はこれを見て，困惑しているところである．いろいろ思うところはあるが，とりあえず抜粋すると，

コメントがない
行が空いていない
処理ごとに変数が依存しあっている

コメントがない

私の悪い癖として，書けと指示されなかったらソースコードにコメントを一切書かないことがある
— うたがわきき🔰💊 (@utgwkk) 2017年1月19日

悪い癖だと思う．自分だけのコードであっても，少し時間が経つと意図が不明になるというのはよくあることで，この条件式はこういう意味とかをとりあえず書いておけば，あとでそこを変更することになっても，現状に合うようにするだけで，タイムリープして条件式を解釈しなおす必要がなくなると思う．

行が空いていない

これもあまり良くないと思っている．先のコードは，もう少し噛み砕くと，次のような処理をやっている．

いいねイベントを受け取る
自分のいいねイベントであり，いいねしたツイートにメディアが付いていて，かつツイートした人が限定公開アカウントでないなら次へ
Slack に投稿する用にツイートの URL を生成する
ダウンロードする画像の URL のリスト取得する
URL を出力し，同時に Slack にも投稿する
ダウンロードする画像の URL に対して，次のことを行う
1. ダウンロード先のパスと，サムネイルを保存するパスを生成する
2. 画像をダウンロードする
3. サムネイルを生成して保存する
4. DB に書き込む

これらの処理がぜんぶ，行を空けずに書いてあるので，初めて見た人にはどこまでがどの処理かが分かりづらくなっている．

処理ごとに変数が依存しあっている

私は，たとえば，ダウンロードとサムネイルの生成は別個で行われるべきだと考える．そうして download というメソッドと generate_thumbnail というメソッドに分けようと思うが，このコードを素直に切り貼りしただけではうまく動かない．サムネイルの生成が，ダウンロードに用いる body という変数(ダウンロードしたファイルのデータを表す)に依存しているのである！

この場合は，サムネイルの生成はダウンロードより後に行われるとしているので，切り分ける際には次のようになるだろうか．

def download(url, save_path)
  begin
    # :orig 付きでダウンロードしてみる
    body = open(url + ':orig', &:read)
  rescue OpenURI::HTTPError
    begin
      # だめなら :orig なし
      body = open(url, &:read)
    rescue OpenURI::HTTPError
      raise '画像のダウンロードに失敗しました'
    end
  end
end

def generate_thumbnail(from_path, to_path)
  File.open(from_path, 'rb') do |f|
    Magick::Image.from_blob(f.read).shift.resize_to_fit(128).write to_path
  end
end

basename = File::basename url

download_path = File::join DOWNLOAD_DIR, basename
download url, download_path

thumbnail_path = File::join THUMBNAIL_DIR, basename
generate_thumbnail download_path, thumbnail_path

まとめ

勢いよく書いて運用しているコードにあとから機能が必要になったと，これまた深夜テンションで継ぎ足していくと，あとで見直したときに行方不明になってしまう．

普段から疎結合なコードを書く癖がないとすぐこうなる，という典型例っぽい気がしてきた．みなさんは疎結合なコードを書くようにしましょう．あとテストも書きましょう．

人間がコードを書く際，常に正常な判断ができる状態であるとは限らないので，どうしても起こるものは起こるものです．問題はそこからどのようにして軌道修正できるか．
— うたがわきき🔰💊 (@utgwkk) 2017年1月19日

常に正常な判断がしたい，正常とは……？

あわせて読みたい

utgwkk.hateblo.jp

私が歌川です

@utgwkk が書いている

深夜テンションで書いたコードを長期間運用していくと発生すること

コメントがない

行が空いていない

処理ごとに変数が依存しあっている

まとめ

あわせて読みたい