外部APIと連携するプログラムのコードレビューしてた時にあったものです。
本のタイトル、著者、値段を取得する仕様になってました。
細かいところはあんまり覚えてない・・・
抜粋するとこんな感じ
book_titles = ["吾輩は猫である", "人間失格", "舞姫"]
book_authors = ["夏目 漱石", "太宰治", "森鴎外"]
book_prices = [1500, 2000, 1700]
book_titles.each_with_index do |book_title, index|
puts "タイトル:#{book_title}, 著者:#{book_authors[index]}, 値段:#{book_prices[index]}円"
end
なにがイケてないってタイトルでループしてそのインデックスを使って他の情報にアクセスしているところ
サンプルみたいな単純なプログラムだとこだわる必要はないが、もう少し処理が複雑になってきたらもう目も当てられない状況に・・・
どうやってbook_titles
、book_authors
、book_prices
を作っていたのか記憶が曖昧なのですが、各配列にちゃんとした意味があるなら1つのクラスとして集約されるべきだと思います。
なので、Struct
を使ってリファクタぽいことしてみました。
book_titles = ["吾輩は猫である", "人間失格", "舞姫"]
book_authors = ["夏目 漱石", "太宰治", "森鴎外"]
book_prices = [1500, 2000, 1700]
Book = Struct.new(:title, :author, :price)
books = []
3.times do |index|
books << Book.new(book_titles[index], book_authors[index], book_prices[index])
end
books.each do |book|
puts "タイトル:#{book.title}, 著者:#{book.author}, 値段:#{book.price}円"
end
メモリ効率的にどうなの?というツッコミは置いといて、
こういうリファクタがいいのやら、わるいのやら?