概要
国民の祝日.csv
を、使いやすい形式にパースする課題にチャレンジしました。
置換前と置換後は、以下となります。
置換前
app/public_holiday.csv
平成28年(2016年),,平成29年(2017年),,平成30年(2018年),
名称,月日,名称,月日,名称,月日
元日,2016/01/01,元日,2017/01/01,元日,2018/01/01
成人の日,2016/01/11,成人の日,2017/01/09,成人の日,2018/01/08
建国記念の日,2016/02/11,建国記念の日,2017/02/11,建国記念の日,2018/02/11
春分の日,2016/03/20,春分の日,2017/03/20,春分の日,2018/03/21
昭和の日,2016/04/29,昭和の日,2017/04/29,昭和の日,2018/04/29
憲法記念日,2016/05/03,憲法記念日,2017/05/03,憲法記念日,2018/05/03
みどりの日,2016/05/04,みどりの日,2017/05/04,みどりの日,2018/05/04
こどもの日,2016/05/05,こどもの日,2017/05/05,こどもの日,2018/05/05
海の日,2016/07/18,海の日,2017/07/17,海の日,2018/07/16
山の日,2016/08/11,山の日,2017/08/11,山の日,2018/08/11
敬老の日,2016/09/19,敬老の日,2017/09/18,敬老の日,2018/09/17
秋分の日,2016/09/22,秋分の日,2017/09/23,秋分の日,2018/09/23
体育の日,2016/10/10,体育の日,2017/10/09,体育の日,2018/10/08
文化の日,2016/11/03,文化の日,2017/11/03,文化の日,2018/11/03
勤労感謝の日,2016/11/23,勤労感謝の日,2017/11/23,勤労感謝の日,2018/11/23
天皇誕生日,2016/12/23,天皇誕生日,2017/12/23,天皇誕生日,2018/12/23
置換後
{
2016 => {
#<Date: 2016-01-01 ((2457389j,0s,0n),+0s,2299161j)> => '元日',
#<Date: 2016-01-11 ((2457399j,0s,0n),+0s,2299161j)> => '成人の日',
# ...
#<Date: 2016-11-23 ((2457399j,0s,0n),+0s,2299161j)> => '勤労感謝の日',
#<Date: 2016-12-23 ((2457399j,0s,0n),+0s,2299161j)> => '天皇誕生日',
},
2017 => {
#<Date: 2017-01-01 ((2457399j,0s,0n),+0s,2299161j)> => '元日',
#<Date: 2017-01-09 ((2457399j,0s,0n),+0s,2299161j)> => '成人の日',
# ...
#<Date: 2017-11-23 ((2457399j,0s,0n),+0s,2299161j)> => '勤労感謝の日',
#<Date: 2017-12-23 ((2457399j,0s,0n),+0s,2299161j)> => '天皇誕生日',
},
2018 => {
#<Date: 2018-01-01 ((2457399j,0s,0n),+0s,2299161j)> => '元日',
#<Date: 2018-01-08 ((2457399j,0s,0n),+0s,2299161j)> => '成人の日',
# ...
#<Date: 2018-11-23 ((2457399j,0s,0n),+0s,2299161j)> => '勤労感謝の日',
#<Date: 2018-12-23 ((2457399j,0s,0n),+0s,2299161j)> => '天皇誕生日',
},
}
このような形式のハッシュに置換するために自分が書いたコードと、回答例のコードを比較して、ポイントをまとめていきたいと思います。
自分が書いたソースコード
app/public_holiday_converter.rb
require 'csv'
require 'date'
class PublicHolidayConverter
def initialize
@data_list = CSV.read('app/public_holiday.csv')
end
def call
pair_array = @data_list.flatten.each_slice(2).map { |a| a }.drop(6)
pair_hash = pair_array.map do |a, b|
Hash[Date.parse(b), a]
end
name_hash = pair_hash.each_slice(3).map { |a| a }
transform_hash(name_hash)
end
private
def transform_hash(name_hash)
year_hash1 = { 2016 => name_hash.map { |a| a[0].flatten }.to_h }
year_hash2 = { 2017 => name_hash.map { |a| a[1].flatten }.to_h }
year_hash3 = { 2018 => name_hash.map { |a| a[2].flatten }.to_h }
hash = {}
hash.merge(year_hash1).merge(year_hash2).merge(year_hash3)
end
end
自分で解いてみた結果です。
次に出てくる回答例とは程遠いですね。。
置換後の形式にパースされてはいますが、全体的に可読性が低いと感じます。
良くなさそうな箇所をリストアップしてみます。
- 全体的に汎用性がない作りになっている。
- 年が増えた場合に、この実装では対応出来ていない。
- 変数名が適切でない。
-
array
が入っているのに、_hash
という変数名が付いている。 -
a
やb
という変数名では、中身が想像できない。(map
のブロック引数)
-
- 冗長な記述や、無駄な記述がある。
-
year_hash1
等、同じような処理を書いている。 -
hash = {}
は不要。
-
-
public_holiday.csv
の最初の行を活用できていない。- 上手く使用できず
drop(6)
で捨ててしまっている。
- 上手く使用できず
回答例のソースコード
※ 先輩にいただいた回答例です。
app/public_holiday_converter.rb
require 'csv'
require 'date'
class PublicHolidayConverter
HOLIDAY_ROW_START_POINT = 2
def initialize
@data_list = CSV.read('app/public_holiday.csv')
end
def call
holiday_infos_array.each_with_object({}) do |holiday_info_of_year, hash|
year = year(holiday_info_of_year[0][0])
holiday_info_of_year[HOLIDAY_ROW_START_POINT..].each do |holiday_info_of_day|
hash[year] ||= {}
hash[year][Date.parse(holiday_info_of_day[1])] = holiday_info_of_day[0]
end
end
end
private
def holiday_infos_array
@data_list.transpose.each_slice(2).map(&:transpose)
end
def year(date_text)
date_text[/\d{4}/].to_i
end
end
私が気付けなかったことや困っていたところが解決されているので、良いと思ったポイントをリストアップしたいと思います。
- 固定値が定数化されている。(
HOLIDAY_ROW_START_POINT
) -
each_with_object
を使用している。- ハッシュで値が格納されるというイメージがつきやすい。
- 変数名が分かりやすい。
- メソッドがそれぞれの役割に分かれているので、返ってくる値のイメージがつきやすい。
-
public_holiday.csv
の最初の行が使用されている。- 正規表現で4文字の数字を取得している。
まとめ
最初に課題を見たときは、それほど難しくなさそうに思いましたが、いざやってみるとなかなか思うように行かず、苦戦してしまいました。。
繰り返しの処理が多かったので、Enumerable
のメソッドともっと仲良くならないといけないなと感じました。
今回学んだポイントを生かして、可読性の高いコードが書けるようになりたいと思います。