Help us understand the problem. What is going on with this article?

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

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を使っていたりと目からウロコのやり方のコードを見れて勉強不足の身ながら感心しております。
みんなで一つの課題に対してコードを書いてお互いに見せ合うみたいな勉強会参加したいなぁ(欲望)ということで今回の記事はおしまいです。
最後まで読んでいただきありがとうございます。

Why do not you register as a user and use Qiita more conveniently?
  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
Comments
Sign up for free and join this conversation.
If you already have a Qiita account
Why do not you register as a user and use Qiita more conveniently?
You need to log in to use this function. Qiita can be used more conveniently after logging in.
You seem to be reading articles frequently this month. Qiita can be used more conveniently after logging in.
  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