読者です 読者をやめる 読者になる 読者になる

CodeIQ の解答例を Ruby っぽくする

エンジニア夏祭り2013「オラにじぇじぇじぇなコードを書いてくんろ!」解答編 の解答例の Ruby コードがあまり Ruby っぽくなかったので、勝手に Ruby らしくしてみます。

元の解答例

s="gtgtsgipgttptinggipsppaigsesgpetgstpatetisiesagaeaigttetepitiatsegssieeeeatepaaiagtpieataatppiitgiapsteitatiiatpetetetttgpetpaasipttssstpeeeggtiagtttegtiipestsasgpsepaasapttgattgiatppegitiatpasgatgepttggapesaeetaeissttggieietgspagesiipestipggstttpateptitiaetottissgggtttaipappgstsptttgtpispattgegstltiappseisapgistaiagteeiptptpisaieisagstapeteietgteiisgtiptstgtstasspeatspptitttatteastsgtptgtasggpniaaeteaisett"
n='neapolitan'.split(//)
i=0
a=""
s.split(//).each{|x|n[i]==x ?(i+=1;a<<"[#{x}]"):a<<x}
print a

この解答例が Ruby っぽくないと感じた一番大きな理由は、ループ処理の外側でカウンタ変数を初期化して、ループの中でカウントアップしてることです。

ループ変数をなくしてみます。

s="gtgtsgipgttptinggipsppaigsesgpetgstpatetisiesagaeaigttetepitiatsegssieeeeatepaaiagtpieataatppiitgiapsteitatiiatpetetetttgpetpaasipttssstpeeeggtiagtttegtiipestsasgpsepaasapttgattgiatppegitiatpasgatgepttggapesaeetaeissttggieietgspagesiipestipggstttpateptitiaetottissgggtttaipappgstsptttgtpispattgegstltiappseisapgistaiagteeiptptpisaieisagstapeteietgteiisgtiptstgtstasspeatspptitttatteastsgtptgtasggpniaaeteaisett"
n='neapolitan'.split(//)
a=""
s.split(//).each{|x| n[0]==x ? a<<"[#{n.shift}]" : a<<x}
print a

あと、三項演算子の中で同じ処理(a<<)を行なっているのも気になります。条件によって値が異なるだけであれば、その値を用いた処理は三項演算子の外でできます。

s="gtgtsgipgttptinggipsppaigsesgpetgstpatetisiesagaeaigttetepitiatsegssieeeeatepaaiagtpieataatppiitgiapsteitatiiatpetetetttgpetpaasipttssstpeeeggtiagtttegtiipestsasgpsepaasapttgattgiatppegitiatpasgatgepttggapesaeetaeissttggieietgspagesiipestipggstttpateptitiaetottissgggtttaipappgstsptttgtpispattgegstltiappseisapgistaiagteeiptptpisaieisagstapeteietgteiisgtiptstgtstasspeatspptitttatteastsgtptgtasggpniaaeteaisett"
n='neapolitan'.split(//)
a=""
s.split(//).each{|x| a << (n[0]==x ? "[#{n.shift}]" : x)}
print a

文字列を文字ごとに分割するのに split(//) を使用していますが、Ruby 2.0 では String#chars が使えるので、それを使った方がいいと思います。

String#charsRuby 1.8.7 や 1.9.3 にもありますが、2.0 とは動きが異なるので、注意が必要です。

s="gtgtsgipgttptinggipsppaigsesgpetgstpatetisiesagaeaigttetepitiatsegssieeeeatepaaiagtpieataatppiitgiapsteitatiiatpetetetttgpetpaasipttssstpeeeggtiagtttegtiipestsasgpsepaasapttgattgiatppegitiatpasgatgepttggapesaeetaeissttggieietgspagesiipestipggstttpateptitiaetottissgggtttaipappgstsptttgtpispattgegstltiappseisapgistaiagteeiptptpisaieisagstapeteietgteiisgtiptstgtstasspeatspptitttatteastsgtptgtasggpniaaeteaisett"
n='neapolitan'.chars
# n='neapolitan'.chars.to_a   # for Ruby 1.8.7/1.9.3
a=""
s.chars.each{|x| a << (n[0]==x ? "[#{n.shift}]" : x)}
print a

最後に、結果を格納する変数 a をループの外で初期化して、ループ内で追加してるのが気になります。この変数も無くしてみます。

s="gtgtsgipgttptinggipsppaigsesgpetgstpatetisiesagaeaigttetepitiatsegssieeeeatepaaiagtpieataatppiitgiapsteitatiiatpetetetttgpetpaasipttssstpeeeggtiagtttegtiipestsasgpsepaasapttgattgiatppegitiatpasgatgepttggapesaeetaeissttggieietgspagesiipestipggstttpateptitiaetottissgggtttaipappgstsptttgtpispattgegstltiappseisapgistaiagteeiptptpisaieisagstapeteietgteiisgtiptstgtstasspeatspptitttatteastsgtptgtasggpniaaeteaisett"
n='neapolitan'.chars
print s.chars.map{|x| n[0]==x ? "[#{n.shift}]" : x}.join

これでかなり Ruby っぽいプログラムになったと思いますが、どうでしょう。