Ruby
オブジェクト指向
リファクタリング
oop

RubyでRubyっぽいプログラムを書いてみる

RubyでRubyっぽく(OOPっぽく)プログラムを書いてみます。

修正履歴

  • 2018/07/11
    • scivolaさんにご指摘いただき、その4: エラー処理の追加内のハッシュへのfreeze処理を修正しました。

背景

お仕事している中で、一つのメソッドの中で数十行の処理が書かれていたり、クラスやメソッドに直接関係のない要素をその中で定義していたりするコードに遭遇することが多々あります。
今この瞬間に動くアプリケーションを作ることが目的であればそれでも良いのかもしれませんが(私はそれでも反対ですが)、、、現実世界では状況が刻々と変わります。

上記のような、今この瞬間に動くアプリケーションは往往にして抽象度の低いコードで書かれており、ある目的だけを果たすために書く分にはラクですが、変更に対処できません。
エンジニアとして、ただ動けば良いというもののもう一歩先まで考えられるようになりたいなと思い、勉強のアウトプットをまとめました。

前提

ユーザー情報を返すWebAPIに対してユーザーIDを指定してGETし、該当ユーザーの情報を取得する処理を作る。
GET時に返される値は下記のような感じ
{user_id: 1, name: 'hoge太郎', tel: '090-1111-2222'}

その1:ベタ書き

何はともあれ動かないといけないので、ひとまず書きます。

user.rb
class User
  require 'net/http'
  require 'uri'
  require 'json'

  def initialize(user_id)
    @user_id = user_id
  end

  def show_user_info
    uri = "https://friends-hogehoge.com/user/#{@user_id}"
    uri = URI.parse(uri)
      response = Net::HTTP.start(uri.host, uri.port, use_ssl: uri.scheme === 'https') do |http|
        http.get(uri.request_uri)
      end
      result = JSON.parse(response.body)
        puts "user_info: #{result}"
    end    
  end
end

課題

ひとまず動くようになりましたが、下記のような課題が生まれました。

1. 一つのメソッドの中にやりたいことが全部入っている
  - GETメソッドの処理はUser内固有のものではない
    - 外に切り出す
2. URLが処理の中にベタがき
  - baseのURLとかは他の処理でも使うし、各ファイルにベタがきしていると更新漏れやダブりが発生する
    - 定数として一箇所にまとめよう

etc.(ツッコミどころは他にもたくさんあるかと思いますが...)

動けばいいというものではない、というわけなので、修正を加える。

その2: 処理の分割

show_user_info内でやっている処理をバラしてやります。
ついでに、定数とかはfreezeして荒らされないようにします。

user.rb
class User
  require 'net/http'
  require 'uri'
  require 'json'

  def initialize(user_id)
    @user_id = user_id
  end

  def uri_info
    {
      base_uri: "https://friends-hogehoge.com",
      path_to_user: "/user/"
    }.map(&:freeze).to_h.freeze
  end

  def show_user_info
    uri = uri_info[:base_uri] + uri_info[:path_to_user] + @user_id.to_s
    result = get_resource(uri)
    puts result
  end

  private
    def get_resource(uri)
      uri = URI.parse(uri)
      response = Net::HTTP.start(uri.host, uri.port, use_ssl: uri.scheme === 'https') do |http|
        http.get(uri.request_uri)
      end
      JSON.parse(response.body)
    end
end

課題

show_user_infoはスッキリましたが、目線を上げてUserクラスを見ると、Userクラスが担わなくても良い処理が多々あります。
これをまた分解してやります。

その3: さらなる処理の分割

user.rb
class User

  def initialize(user_id)
    @user_id = user_id
  end

  def show_user_info
    uri = Router::URI_INFO[:base_uri] + Router::URI_INFO[:path_to_user] + @user_id.to_s
    result = Router::get_resource(uri)
    puts result
  end
end

module Router
  require 'net/http'
  require 'uri'
  require 'json'

  URI_INFO = {
    base_url: "https://freundschaft.herokuapp.com",
    path_to_user: "/user/"
  }.map(&:freeze).to_h.freeze

  class << self
    def get_resource(uri)
      uri = URI.parse(uri)
      response = Net::HTTP.start(uri.host, uri.port, use_ssl: uri.scheme === 'https') do |http|
        http.get(uri.request_uri)
      end
      JSON.parse(response.body)
    end
  end
end

その4: エラー処理の追加

ひとまず、コードはスッキリしました。
処理の所属する場所も、以前よりはあるべき場所にあるかんじになりました。
ここでもう一歩、エラー処理を考えてみましょう。
今回は、GETのレスポンスのSTATUSが2xx以外だった場合に例外処理を入れてやります。

user.rb
class User

  def initialize(user_id)
    @user_id = user_id
  end

  def show_user_info
    uri = Router::URI_INFO[:base_uri] + Router::URI_INFO[:path_to_user] + @user_id.to_s
    result = Router::get_resource(uri)
    puts result
  end
end

module Router
  require 'net/http'
  require 'uri'
  require 'json'

  URI_INFO = {
    base_url: "https://freundschaft.herokuapp.com",
    path_to_user: "/user/"
  }.transform_values(&:freeze).freeze

  class << self
    def get_resource(uri)
      uri = URI.parse(uri)
      begin
        response = get_response(uri)
        case response
        when Net::HTTPSuccess
          JSON.parse(response.body)
        else
          error_message(uri, response.value)
          exit
        end
      rescue Net::HTTPServerException => err
        error_message(uri, err)
        exit
      end
    end

    private
      def get_response(uri)
        response = Net::HTTP.start(uri.host, uri.port, use_ssl: uri.scheme === 'https') do |http|
          http.open_timeout = 5
          http.read_timeout = 10
          http.get(uri.request_uri)
        end
      end

      def error_message(uri, response)
        puts "[ERROR] Could not get resources with status 2xx."
        puts "[LOG] URI: #{uri.to_s}, ERROR MESSAGE: #{response}"
      end
  end
end

雑感

  • ひとまず、動くコードを書いた上で、このクラス/メソッドは役割をたくさん持ちすぎていないかな?と自問していくのが、リファクタリングやOOPっぽく書く第一歩なのかなと思いました。

参考