Qiita Teams that are logged in
You are not logged in to any team

Log in to Qiita Team
Community
OrganizationAdvent CalendarQiitadon (β)
Service
Qiita JobsQiita ZineQiita Blog
1
Help us understand the problem. What is going on with this article?
@QWYNG

自分で書いたコードを自分でリファクタリング

More than 1 year has passed since last update.

はじめに

最近「プロを目指す人のためのruby入門」を読み直しているものです。

[Ruby][解答募集]文字列に出現する単語の個数を数えてみよう

三ヶ月くらい前にこちらの記事を読んで自分が書いたコードが汚く見えてきたので自分が今まで学んだことでできる範囲でリファクタリングしてみました。
まずは元のコード

def count_word(string)
  words_array = string.split(' ')
  uniq_words = words_array.uniq
  hash = {}
  uniq_words.each do |word|
    hash[word] = words_array.count(word)
  end
  hash
end

リファクタリングしようと思えばより高速で短くする方法はいくらでもあると思いますが、今回はローカル変数の宣言が多いという問題点に絞ってロジックそのものは変えずその点を改良できるようなリファクタリングを行いました。

uniq_array

uniq_arrayを定義しなくても、hash[word]で同じキーの要素があれば更新されるのでいらないのでは、と考え、まずこの宣言を削除。このリファクタリングだと同じ単語の数だけ処理が行われてしまうので、よりよいリファクタリングはまだまだ追求できそう。

def count_word(strings)
  words_array = strings.split(' ')
  hash = {}
  words_array.each do |word|
    hash[word] = words_array.count(word)
  end
  hash
end

hash = {}

このハッシュの宣言も折りたたみ演算としてinjectを使えば削除できます。

  words_array.inject(Hash.new) do |hash, word|
    hash[word] = words_array.count(word)
    hash
  end

最終的に

最後になりましたがcount_wordというメソッド名も良くないですね。count_by_wordのほうが何をするメソッドなのかわかりやすいです

def count_by_word(string)
  words_array = string.split(' ')
  words_array.inject(Hash.new) do |hash, word|
    hash[word] = words_array.count(word)
    hash
  end
end

結局まだ一回変数宣言してますね… group_byしたりハッシュのバリューを増やしていく方法を使えば省略できそうですが今回はメインのロジックは変えない形でのリファクタリングなのでこれ以降はなかなか思いつきませんでした。もしまだまだイケる!という方いればぜひご教授ください。

まとめ

リファクタリングする際には昔書いたテストが役に立ちました。やっぱりテストは大事。
課題の記事のコメント欄にある回答例をみるとtapを使っていたり、group_byを使っていたりと目からウロコのやり方のコードを見れて勉強不足の身ながら感心しております。
みんなで一つの課題に対してコードを書いてお互いに見せ合うみたいな勉強会参加したいなぁ(欲望)ということで今回の記事はおしまいです。
最後まで読んでいただきありがとうございます。

1
Help us understand the problem. What is going on with this article?
Why not register and get more from Qiita?
  1. We will deliver articles that match you
    By following users and tags, you can catch up information on technical fields that you are interested in as a whole
  2. you can read useful information later efficiently
    By "stocking" the articles you like, you can search right away
QWYNG
globis
グロービスは 1992 年の創業以来、社会人を対象とした MBA、人材育成の領域で Ed-Tech サービスを提供し、現在は日本 No.1 の実績があります。これらの資産と、さらに IT や AI を活用することで、アジア No.1 を目指しています。

Comments

No comments
Sign up for free and join this conversation.
Sign Up
If you already have a Qiita account Login
1
Help us understand the problem. What is going on with this article?