マネーフォワード社内PRに見られるRubyの書き方について (3)

エンジニアの澤田です。

この連載では、マネーフォワード社内のRuby (on Rails)コードで気になった箇所の問題点やそこから発展して関連事項を議論しています。

前回の『マネーフォワード社内PRに見られるRubyの書き方について(2)』ではハッシュの生成を扱いました。
概念的な話で始まり、また長かったので、読んだ方は少し疲れたかも知れません。

今回は内容の特性により、用例を並べて手短に問題点を指摘して、文字列(String)の生成や検証を考察します。

題材とするコードは、マネーフォワード社内のGitHubプルリクエストで実際に見かけたコードから問題点に関係する部分を抽出し、抽象化したもので、見かけたものそのままではありません。

文字列の生成や検証

社内のコードに限らず、文字列に関わるRubyコードで問題のあるものの多くは、必要もないのに正規表現を使ってやろうとしていたり、特定のメソッドに固執してそれを乱用しているということに集約されると思います。正規表現の乱用の例とメソッド別の不的確な用例を挙げていきます。

正規表現の乱用

単一の文字列を表している

String#splitString#subなどの特定のメソッドの引数にやたらと正規表現を使っている例を見ます。

[見かけた書き方]
string.split(/fixed pattern/)
string.sub(/fixed pattern/, "bar")

正規表現は、あるパターンに従う文字列の集まりを指したいときには便利ですが、単一の文字列とのマッチに正規表現を使うのはやめましょう。文字列を使うべきです。実行速度がだいぶ違います。

string.split("fixed pattern")
string.sub("fixed pattern", "bar")

String#=~Regexp#=~への固執

真偽値が欲しいだけ

パターンマッチするか否かということを表す真偽値(true, false)、もしくは真っぽい(truthy)か偽っぽい(falsy)かの区別をするだけで十分なのに、何でもString=~Regexp=~を使っている例があります。

[見かけた書き方]
if string =~ /pattern/

=~はマッチに成功した場合にただのtrueではなくマッチの位置を返します。制御構造の条件や論理演算で使っても問題になることはありませんが、これは欲しい情報に対して過多な情報で、そのために実行効率を落としています。真偽値を返すString#match?Regexp#match?に置き換えるだけで数倍速くなります。

if string.match?(/pattern/)

完全一致

次のようなコードで文字列の完全一致を調べようとしているのを見かけます。パターンマッチには=~と考えてしまうのでしょうか。

[見かけた書き方]
string =~ /\Afixed pattern\z/

ちなみに、文字列の先頭、末尾のつもりで行頭、行末記号を使っているコードも見かけます。

[見かけた書き方]
string =~ /^fixed pattern$/

後者は完全一致のつもりのコードとしては完全に間違いです。正しく\A^\z$を使い分けましょう。

完全一致の場合は=~(もしくはmatch?)で比較するまでもなく、文字列にとって最も基本的な関係演算である==を使えばよいのです。

string == "fixed pattern"

さらに、== stringによる完全一致とmatch?(regexp)による部分一致の条件が以下のように混在するときには、

if string == "fixed pattern"
  ...
elsif string.match?(/pattern/)
  ...
elsif string == "another fixed pattern" or string.match?(/another pattern/)
  ...
end

case文の中でレシーバーと引数の立場を入れ替え、string ===regexp ===として統一して扱えます。

case string
when "fixed pattern"
  ...
when /pattern/
  ...
when "another fixed pattern", /another pattern/
  ...
end

先頭や末尾での一致

文字列の先頭もしくは末尾でのマッチを調べる場合にも上と同様のコードが書かれていることがあります。

[見かけた書き方]
string =~ /\Achapter\n/
string =~ /\.html\z/

==が使えないので=~match?を使いがちになってしまうのかも知れませんが、このようなときにも、それ専用のメソッドString#start_with?String#end_with?があります。これらを使えば正規表現も要りません。しかも、これらのメソッドでは、単一の文字列だけでなく、複数の文字列を引数に与えることが出来ます。

string.start_with?("chapter\n", "section\n")
string.end_with?(".html", ".htm", ".js")

しかも、String#start_with?String#end_with?よりも柔軟性があり、必要とあれば、正規表現も引数に取ることが出来ます。

string.start_with?("chapter\n", /(sub)section\n/)

肯定形ばかり使っている

「~という文字を含んでいないか」を調べるために、「全体が~以外の文字で出来ているか」という間接的な表現を使っているコードがよくあります。例えば、stringが数字以外の文字を含んでいないことを保証する条件として次のようなコードを見かけます。

[見かけた書き方]
string =~ /\A\d*\z/

これでも正しく動きますが、メソッドの否定を使って正規表現を分かりやすくすることが出来ます。

string !~ /\D/

ただし、!~は真偽値を返すものの、内部で=~!を呼び出していて、=~と同様に遅いです。もし実行速度が最優先なら、上に述べたmatch?を使って

string.match?(/\A\d*\z/)

とするのが最善です。分かりやすさを取るなら!~、速度を取るならmatch?という使い分けをするのが良いでしょう。

Enumerableからの選択や除去

Enumerableなオブジェクトからの正規表現マッチによる選択や除去に=~を明示的に使っているコードがあります。

[見かけた書き方]
array # => ["foo_1", "foo_2", "bar_1", "bar_3"]
array.select{|s| s =~ /1/} # => ["foo_1", "bar_1"]
array.reject{|s| s =~ /1/} # => ["foo_2", "bar_3"]

これもmatch?を使えば速くなります。

array.select{|s| s.match?(/1/)} # => ["foo_1", "bar_1"]
array.reject{|s| s.match?(/1/)} # => ["foo_2", "bar_3"]

速さで劣りますが、簡潔に書くにはEnumerable#grepEnumerable#grep_vがあります。

array.grep(/1/) # => ["foo_1", "bar_1"]
array.grep_v(/1/) # => ["foo_2", "bar_3"]

String#splitの乱用

文字や行への分解

splitも乱用されることの多いメソッドです。ありがちなのが、文字列の文字や行への分解やイテレーターの生成に使うことです。

[見かけた書き方]
string # => "abc\ndef\n"
string.split(//).each{...}
string.split("").each{...}
string.split("\n").each{...}

上2つでは文字列を文字に分解してからイテレーションしています。1つ目の空正規表現を使ったものは、上に述べたように、空文字列を使う2つ目のものひどいコードですが、なぜか2つ目よりも使われることが多いです。3つ目は行に分解してイテレーションしています。

文字列を文字や行に分解して配列を得るなら、String#charsString#linesを使って次のように出来ます。

string.chars # => ["a", "b", "c", "\n", "d", "e", "f", "\n"]
string.lines # => ["abc\n", "def\n"]

しかし、これにeachを続けて、

string.chars.each{...}
string.lines.each{...}

とイテレーションするのは無駄です。イテレーションが目的なら、配列を生成する必要がありません。String#each_charString#each_lineで直接イテレーションできます。

string.each_char{...}
string.each_line{...}

必要を超えて分解している

文字列の一部分を抽出したいだけなのに、乱暴にsplitを使って、無駄に分断しているのがよくあります。

[見かけた書き方]
string # => "foo##bar##baz##qux"
first = string.split("##").first # => "foo"
last = string.split("##").last # => "qux"

firstでは"##"で分断されるうちの初めのもの、lastでは最後のものだけに興味があります。それなのに、上のコードではもとの文字列を必要のないところでも分断し、得られた4つの部分のうちの最初か最後だけを使うという無駄をしています。初めの部分を取るには、もとの文字列を左から2つに分断していったら、後はもう分断する必要がないので、第2引数で制限を加えるべきです。

first = string.split("##", 2).first # => "foo"

やや面倒くさい正規表現を使うことにすれば、String#[]を使ってより直接的にまた高速にすることも出来ます。

first = string[/(?:(?!##).)+/] # => "foo"

分断パターンが1文字だけの場合に限っては、正規表現が幾分単純になります。

"foo#bar#baz#qux"[/[^#]+/] # => "foo"

それでも、これらの正規表現はあまりきれいではありません。String#partitionを使えはString#[]と正規表現を使う方法と同等の速さです。

first = string.partition("##").first # => "foo"

一方、もとの文字列から分断された最後の部分を取り出すには、splitでは右側から分断していくオプションはなく、またそのようなことをする対応するメソッドはありません。正規表現も使えません。そこで、右からの場合に使えるのがString#rpartitionです。

last = string.rpartition("##").last # => "qux"

マッチさせる部分とマッチさせない部分が逆

splitはパターンにマッチしなかった部分文字列(とマッチした部分文字列)の配列を返します。このメソッドにはほぼ反対の振る舞いをするString#scanがあり、それはパターンにマッチした部分文字列の配列を返します。状況に応じて、どちらのメソッドを使うべきか選択できます。それなのに、なぜかsplitが偏ってよく使われているようです。

[見かけた書き方]
strings # => ["2019年2月1日", "2019-2-1", "2019/2/1"]
strings.map{|s| s.split(/[年月日\/-]/)} # => [["2019", "2", "1"], ["2019", "2", "1"], ["2019", "2", "1"]]

抽出する部分と捨てる部分のマッチパターンの複雑さに差がある場合、単純な方にマッチする正規表現を書くべきです。それに応じてsplitscanから適切な方を使います。ここでは多様な形式の日付で登場する区切り文字に対応するよりは数字にマッチする方が楽なので、scanを使います。

strings.map{|s| s.scan(/\d+/)} # => [["2019", "2", "1"], ["2019", "2", "1"], ["2019", "2", "1"]]

また、抽出する部分と捨てる部分のマッチパターンの複雑さに差がない場合でも、一緒に使う他のメソッドなどの状況によって、splitscanの一方が他方より優れていることがあります。例えば、次の例では、空白以外の部分文字列を抽出しようとしています。

[見かけた書き方]
string # => " \n foo  bar \t\tbaz\n"
string.strip.split(/\s+/) # => ["foo", "bar", "baz"]

ここではsplitの前にstripを行っています。それがないと、

string.split(/\s+/) # => ["", "foo", "bar", "baz"]

となり、余計な空文字列が混ざってしまうからです。文字列の先頭や末端でのマッチをどう扱うかについては、メソッドによっては面倒な仕様があり、気をつけないといけません。一方、このマッチパターンの補集合である空白文字以外の文字列は、空白文字の文字列/\s+/と同等の複雑さの正規表現/\S+/で表すことが出来、それとscanを使えば、文字列の先頭や末端でのマッチを気にする必要がなくなります。

string.scan(/\S+/) # => ["foo", "bar", "baz"]

配列を経由して文字列に戻っている

文字列から文字列に変換するときに、不必要に配列を経由しているコードがよくあります。

[見かけた書き方]
string # => "foo, bar, baz"
string.split(", ").join("|") # => "foo|bar|baz"

区切り文字列を置き換えるのなら、String#gsubを使って文字列の範囲内で操作すれば良いです。

string.gsub(", ", "|") # => "foo|bar|baz"

文字列の範囲内のことに不用意に配列を巻き込む必要はありません。その昔、高木貞治という数学者が、それまで積分を使って証明されていた微分の定理を微分の範囲内で証明したときに、「微分のことは微分でせよ」と述べたというのは、ある種の人にとってはよく聞く話ですね。

String#gsubの乱用

1文字のパターンをばらばらに繰り返し使っている

gsubを繰り返し使って1文字のパターンの置換を繰り返しているコードがよく出てきます。

[見かけた書き方]
string # => "foo\\bar\n¥baz'qux"
string.gsub("\\", "\").gsub("¥", "¥").gsub("'", "’").gsub("\n", " ") # => "foo\bar ¥baz’qux"

パターンが1文字なら、gsubよりもString#trを使う方が効率的です。しかも、trはマッチ文字と置き換え文字を対応させてそれそれ1つの文字列に書くことが出来ます。

string.tr("¥'\n\\", "¥’ \") # => "foo\bar ¥baz’qux"

特に、1文字のパターンを除去する場合にはString#deleteが使えます。

string.delete("¥'\n\\") # => "foobarbazqux"

マッチが繰り返さない

1回しかマッチしないのに、置換といえばgsubとでも言うかのように、考えずにgsubを使っているコードがあります。

[見かけた書き方]
string # => "test_foo.htm"
string.gsub(".htm", ".html") # => "test_foo.html"

1度しかマッチしない場合や繰り返したくない場合はString#gsubを使うべきではありません。代わりにString#subを使えます。

string.sub(".htm", ".html") # => "test_foo.html"
"test_test_foo.htm".sub("test_", "@") # => "@test_foo.htm"

先頭や末尾のマッチを除去するのなら、Ruby 2.5から追加されたString#delete_prefixString#delete_suffixを使うのが良いです。

string.delete_prefix("test_") # => "foo.htm"
string.delete_suffix(".htm") # => "test_foo"

破壊的バングメソッドの乱用

メソッド連鎖の中

特にStringについては、 バング(!)で終わるメソッドを不必要に使い、変更がなかった場合にnilが返ってきて問題を起こすという場合があります。これはマネーフォワード社内のコードというよりは、プログラマ用の質問サイトなどで別のサービスの開発者が陥っているのをよく見かけます。

[見かけた書き方]
string # => "foo 1"
string.strip!.capitalize!.squeeze!
# >> NoMethodError: undefined method `capitalize!' for nil:NilClass
string # => "foo 1"

バングは危険を伴うメソッドに付けられる習わしになっています。Stringのバングメソッドの内、以下のメソッドは、レシーバーに破壊的変化をもたらします。適用の結果、レシーバーに変化がなければnilを返します。

String#capitalize!, String#chomp!, String#chop!, String#delete!, String#delete_prefix!, String#delete_suffix!, String#downcase!, String#gsub!, String#lstrip!, String#rstrip, String#slice!, String#squeeze!, String#strip!, String#sub!, String#swapcase!, String#tr!, String#tr_s!, String#upcase!

これらのメソッドによるメソッド連鎖の最後以外のどこかで変更がない場合、その箇所で返り値がnilとなり、後続するStringメソッドが呼び出されません。むやみに破壊的バングメソッドを使うのは止めなければなりません。

string # => "foo 1"
string = string.strip.capitalize.squeeze
# => "Fo 1"

ただし、以下のバングメソッドはnilを返さないので、このような心配は無用です。

String#encode!, String#next!, String#reverse!, String#scrub!, String#succ!, String#unicode_normalize!

reverse!について、2003年の変更履歴1 にRuby開発者による「Array#reverse! should not return nil even for arrays sized less than 2.」という記述がありますが、それがどのような理由によるのか、筆者は遡ることが出来ませんでした。他にどのようなメソッドがnilを返さないのかと言うと、筆者が上のリストを見て想像するに、エンコーディングに関わるメソッドであるようです。

このように、メソッド連鎖の中では普通は使えないバングメソッドですが、1つの文字列を連続的に整形していく場合には、途中で1つの操作ごとに新しい文字列を生成しては捨てるということを繰り返すよりは、同一の文字列を破壊的に変更するメソッドを使う方が、実行速度が速いです。大きな文字列を扱ったり文字列の加工を集中的に行うようなRubyプログラムでは、破壊的な変更を行うメソッドの使用は必須です。そのようなときは、格好は悪いですが、命令型プログラミングでメソッドをステップを踏んで順に適用します。

string # => "foo 1"
string.strip!
string.capitalize!
string.squeeze!
string # => "Fo 1"

ちなみに、Kernel#tapを使って破壊的メソッドを連鎖にすることも考えられますが、

string # => "foo 1"
string.tap(&:strip!).tap(&:capitalize!).tap(&:squeeze!)
# => "Fo 1"

実はこれは非破壊的メソッドの連鎖を使う場合よりも却って遅くなってしまうようなので、避けたほうが良さそうです。tapはけっこう重いのですね。

String#chompの乱用

String#to_iの直前

これもマネーフォワード社内のコードよりもプログラマ用の質問サイトなどでよく見かけます。

[見かけた書き方]
string # => "243\n"
string.chomp.to_i # => 243

文字列末尾の空白文字はString#to_iに何の影響もありません。そして、冗長なchompの呼び出しは実行効率を半分近くに下げます。このようなところでchompを使うべきではありません。

string.to_i # => 243

to_sの乱用

Kernel#putsの引数

[見かけた書き方]
puts object.to_s

putsは引数にto_sを適用するので、to_sは冗長です。

puts object

ちなみに、こういうコードも見かけました。

[見かけた書き方]
string # => "foo"
puts "#{string}\n"

文字列の最後に改行がなくて、出力に改行を加えたいという意図のようです。Kernel#printと違ってputsは出力の末尾に改行を加えるので、明示的に改行を加えるのは冗長です。次のようにして同じ結果が得られます。

puts string

式展開の中

[見かけた書き方]
"foo #{object.to_s} bar"

式展開#{}は式に対してto_sを呼び出すので、to_sは冗長です。なくしたほうがずっと読みやすいです。

"foo #{object} bar"

まとめ

文字列の生成や検証については、知っている数少ないメソッドでやり繰りしようとしがちになってしまうのかも知れません。また、文字列加工というと、即座に、正規表現を使うというイメージが湧いてしまうのかも知れません。きちんとRubyの仕様を理解した上で、どのような方法を取るのが合理的なのか冷静に考えながらやっていきたいです。

最後に

マネーフォワードでは、エンジニアを募集しています。
ご応募お待ちしています。

【採用サイト】
マネーフォワード採用サイト
Wantedly | マネーフォワード

【マネーフォワードのプロダクト】
自動家計簿・資産管理サービス『マネーフォワード ME』 iPhone,iPad Android

「しら」ずにお金が「たま」る 人生を楽しむ貯金アプリ『しらたま』 iPhone,iPad

おトクが飛び出すクーポンアプリ『tock pop トックポップ』

金融商品の比較・申し込みサイト『Money Forward Mall』

くらしの経済メディア『MONEY PLUS』

■ビジネス向けクラウドサービス『マネーフォワードクラウドシリーズ』
バックオフィス業務を効率化『マネーフォワードクラウド』
会計ソフト『マネーフォワードクラウド会計』
確定申告ソフト『マネーフォワードクラウド確定申告』
請求書管理ソフト『マネーフォワードクラウド請求書』
給与計算ソフト『マネーフォワードクラウド給与』
経費精算ソフト『マネーフォワードクラウド経費』
マイナンバー管理ソフト『マネーフォワードクラウドマイナンバー』
資金調達サービス『マネーフォワードクラウド資金調達』


  1. https://bugs.ruby-lang.org/projects/ruby-trunk/repository/ruby-git/revisions/776e2693e738975b2bdbeb1ec43e4993a5520665 ↩︎

Pocket