What is the best refactoring? Where should we go from? Difficult for beginners.

There are some redundant parts and some parts I haven't noticed yet, but since I used scope this time, I'll keep them.

When I see myself a few years later, I'm likely to be told, "Is there something I can do better .." (No .. I want to be myself ^^;) To that end, I will learn every day.

Before refactoring:

[Code that obtains latitude and longitude from image metadata and converts them to decimal numbers]

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

#Latitude acquisition
 exif_lat = img.get_exif_by_entry('GPSLatitude')[0][1].split(',').map(&:strip)
#Convert to decimal
 @latitude = (Rational(exif_lat[0]) + Rational(exif_lat[1])/60 + Rational(exif_lat[2])/3600).to_f

#Get longitude
 exif_lng = img.get_exif_by_entry('GPSLongitude')[0][1].split(',').map(&:strip)
#Convert to decimal
 @longitude = (Rational(exif_lng[0]) + Rational(exif_lng[1])/60 + Rational(exif_lng[2])/3600).to_f

... (shame) .. It's so long that I don't want to read it The show action and the confirm action have exactly the same description, so it's even worse ..

Think about how to separate

・ Awareness of the role of the controller and the role of the model ・ The description to convert to decimal number is in service. ・ Shorten the controller side by scoped to acquire latitude and longitude.

Try it like this

Common conversion to decimal numbers

The part separated by exif_lat and exif_lng can be given as an argument, so use exif. The name of the scope is get_exif_gps

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

Described in Post model:
  scope :get_exif_gps, -> (exif){ (Rational(exif[0]) + Rational(exif[1])/60 + Rational(exif[2])/3600).to_f }

Scope the acquisition of latitude

Pass the local variable img as an argument The name of the scope is get_exif_latitude

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

Described in Post model:
  scope :get_exif_latitude, -> (img){ img.get_exif_by_entry('GPSLatitude')[0][1].split(',').map(&:strip) }

Obtain the longitude in the same way.

The name of the scope is get_exif_longitude

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

Described in Post model:
  scope :get_exif_longitude, -> (img){ img.get_exif_by_entry('GPSLongitude')[0][1].split(',').map(&:strip) }

After refactoring

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 model:
  #Get latitude
  scope :get_exif_latitude, -> (img){ img.get_exif_by_entry('GPSLatitude')[0][1].split(',').map(&:strip) }

  #Get longitude
  scope :get_exif_longitude, -> (img){ img.get_exif_by_entry('GPSLongitude')[0][1].split(',').map(&:strip) }

  #Convert to decimal
  scope :get_exif_gps, -> (exif){ (Rational(exif[0]) + Rational(exif[1])/60 + Rational(exif[2])/3600).to_f }

Is it a little easier to follow when you look at the controller? ..

I think it would be a little easier to see if I cut it out into a method, but this time I will leave a memorandum of scope up to this point. I would like to study further and absorb it in the future! !!

reference

https://pikawaka.com/rails/scope

Finally

Do more! If you have any advice, please point it out! ^^

Refactoring with scope