scopeを使ったリファクタリング

リファクタリングって何がベストなのか？　どこから行っていこうか？
初学者からすると難しい。

まだ冗長な箇所や気付けていない箇所もありますが、今回scopeを使ったので残しておこうと思います。

数年後の自分が見たら、「もっとよくできる箇所はあるだろ..」と言われそうですが。（いや..言える自分になりたい ^^;）
そのために、コツコツと日々学んでいきます。

リファクタリング前：

【画像のメタデータから緯度経度を取得して10進数に変換しているコード】

img = Magick::ImageList.new(Rails.root.to_s + "/public#{@post.image.url}")

# 緯度取得
 exif_lat = img.get_exif_by_entry('GPSLatitude')[0][1].split(',').map(&:strip)
# 10進数に変換
 @latitude = (Rational(exif_lat[0]) + Rational(exif_lat[1])/60 + Rational(exif_lat[2])/3600).to_f

# 経度取得
 exif_lng = img.get_exif_by_entry('GPSLongitude')[0][1].split(',').map(&:strip)
# 10進数に変換
 @longitude = (Rational(exif_lng[0]) + Rational(exif_lng[1])/60 + Rational(exif_lng[2])/3600).to_f

...（恥）..めちゃくちゃ長ったらしくて読みたく無い程に見難い
showアクションと、confirmアクションに全く同じ記述があるので、更に格好悪い..

どう切り分けるのか考える

・コントローラの役割とモデルの役割を意識
・10進数に変換する記述は供用する
・緯度経度の取得もscopeして、コントローラ側を短くする

こんな感じでやってみる

10進数への変換を共通化させる

exif_latとexif_lngで分けてある箇所は引数で与えてやればいいので、exifとする
スコープの名前はget_exif_gps

app/controller/posts_controller:
  @latitude = Post.get_exif_gps(exif_lat)
  @longitude =  Post.get_exif_gps(exif_lng)

Postモデルに記述：
  scope :get_exif_gps, -> (exif){ (Rational(exif[0]) + Rational(exif[1])/60 + Rational(exif[2])/3600).to_f }

緯度の取得もscopeさせる

ローカル変数imgを引数で渡す
スコープの名前はget_exif_latitude

app/controller/posts_controller:
  exif_lat = Post.get_exif_latitude(img)

Postモデルに記述：
  scope :get_exif_latitude, -> (img){ img.get_exif_by_entry('GPSLatitude')[0][1].split(',').map(&:strip) }

※経度の取得も同様に行う

スコープの名前はget_exif_longitude

app/controller/posts_controller:
  exif_lng = Post.get_exif_longitude(img)

Postモデルに記述：
  scope :get_exif_longitude, -> (img){ img.get_exif_by_entry('GPSLongitude')[0][1].split(',').map(&:strip) }

リファクタリング後

app/controller/posts_controller:

  img = Magick::ImageList.new(Rails.root.to_s + "/public#{@post.image.url}")
  exif_lat = Post.get_exif_latitude(img)
  @latitude = Post.get_exif_gps(exif_lat)

  exif_lng = Post.get_exif_longitude(img)
  @longitude =  Post.get_exif_gps(exif_lng)

Postモデル：
  # 緯度の取得
  scope :get_exif_latitude, -> (img){ img.get_exif_by_entry('GPSLatitude')[0][1].split(',').map(&:strip) }

  # 経度の取得
  scope :get_exif_longitude, -> (img){ img.get_exif_by_entry('GPSLongitude')[0][1].split(',').map(&:strip) }

  # 10進数に変換
  scope :get_exif_gps, -> (exif){ (Rational(exif[0]) + Rational(exif[1])/60 + Rational(exif[2])/3600).to_f }

コントローラを見た時に多少は目で追いやすくなったかな？..

メソッドに切り出したりしたら、もう少し見やすくできそうな気もしますが、今回はここまでをscopeの備忘録がてら残しておきます。
今後も更に勉強して吸収していきたいと思います！！

参考

最後に

もっとこうすれば良い！というアドバイスございましたら、是非ご指摘ください！ ^^