5
1

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 1 year has passed since last update.

[Refactoring]多くなったPrivateメソッドはクラス化できる!!

Last updated at Posted at 2022-06-29

どうも、「病院なび」の開発チームメンバー甘利です。
今日はリファクタリングについて書こうと思います。

うちのホープの@KON-ch(こん)が勉強会に参加して「多くなったPrivateメソッドはクラス化できる!!」というのをもたらしてくれました。
確かにそうだわ、、と納得したので早速使っていきたいと思います。

そもそもなぜPrivateメソッド達をクラスにする必要があるのか

単体テストの範囲を細かくできます

Privateメソッドがたくさんある実装

以前書いた 自社サービス「病院なび」へ AWS Opensearch を導入したいを実装した際のクラスをリファクタリングしたいと思います。このIndexingServiceというクラスは社内での運用に合わせて Opensearch のインデックスを簡単に管理する目的で実装しています。簡単に管理する一環としてIndexを作成する際に、指定されたインデックス名に日付を表すサフィックスを付与しindex_name_20220624の様な名前にして作成します。このサフィックスを生成するメソッドはプライベートメソッドとして実装されています。

 indexing_service.rb
class IndexingService
  def create(index_prefix)
    @client.indices.create(
      index:  index_with_time(index_prefix),
      body:   schema_json(index_prefix)
    )
  end       

  private

  def time_suffix
    Time.now.strftime("%Y%m%d-%H%M%S")
  end

  def index_name(name_prefix)
    [name_prefix, time_suffix].join("_")
  end
end

よくよく考えなくても、このindex_name はただ名前を表すStringを返すので IndexingService になくて良いですよね。
ということで、Class化していきたいと思います。

Class化してみ

無事 IndexingServiceindex_name メソッドが見事 IndexingService::Name のパブリックメソッドとなりました。
(テストコードなんていらない!!とは t-wada さんの前では言えないので、見つからないことを祈ります。)

 indexing_service/name.rb
class IndexingService::Name
  def initialize(prefix)
    @prefix = prefix
  end

  def index_name
    [@prefix, time_suffix].join("_")
  end
  
  private

  def time_suffix
    Time.now.strftime("%Y%m%d-%H%M%S")
  end
end

元々の呼び出し元であったIndexingService::Name は以下の様な感じで呼び出しています。
クラスからprivateメソッドをなくなりすっきりしましたね!

 indexing_service.rb
class IndexingService
  def create(index_prefix)
    name = Name.new(index_prefix) #Nameクラスを作るように変更
    @client.indices.create(
      index:  name.index_name,
      body:   schema_json(index_prefix)
    )
  end       
end

Builder化

無事Privateメソッドをクラス化できました。しかし、まだまだ違和感を感じます。
* .index_name メソッドを呼び出した際に日付を取得しているため、同じメソッドを呼んでいても違う値が戻される
* インスタンスを作成する必要がない(Builderパターンにする)
といった点が気になりまね。そこで以下みたいな感じに修正してみす。

 indexing_service/name_builder.rb
class IndexingService::NameBuilder
  def build_with_prefix(prefix)
    [prefix, time_suffix].join("_") 
  end

  private

  def time_suffix
    Time.now.strftime("%Y%m%d-%H%M%S")
  end
end
 indexing_service.rb
class IndexingService
  def create(index_prefix)
    @client.indices.create(
      index:  NameBuilder.build(index_prefix),
      body:   schema_json(index_prefix)
    )
  end       
end

すいっきりしましたね!!!

まとめ

今回はいくつかの`Privateメソッドのクラス化することでリファクタリングを行いました。「Publicメソッドのみテストを書く」という方針で開発をおこなっているため、Privateメソッドが増えてくるとテストしにくいなぁ、、、なんて思っていたのですが、今後は悩まなくても済みそうです。

少しでも面白いと思っていただけたら LGTM お願いします!
またよければフォローいただけると幸いです!!

異論は認めませんがアドバイスは受け付けてます、それでは。

5
1
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
5
1

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?