はじめに

毎月先輩から出していただいた課題に取り組んでいます、 mi0です。
2月は社内報を作成するクラスの実装を行いました。
この記事は要件定義〜レビューをいただくまでの過程を纏めた備忘録です。
こうやったらもっとよくなる、などのご指摘があればコメント頂けると嬉しいです！

過去の記事はこちら！↓

登場人物

私
- なんとなく技術力の上昇を感じ始めた。慢心ではないと思いたい。自作のWebアプリを作ってみたい気持ちが湧いてきた。
小えびのかき揚げ先輩
- 最近自宅の開発環境を整えて嬉しそうにしていらっしゃる。プログラミングが得意。
なんこつ唐揚げちゃん
- 私の心の中の妖精。「やっぱりカロリーは正義よね」が口癖。

要件を定義する

私「今回は社内報を作成するクラスの要件定義から行うよ！初めての試み！」

私「今回は普段使っている社内報を作ってくれるアプリのロジック部分を実装したいんだよね」

私「とりあえず今使ってるアプリがしていることをまとめてみよう」

使用しているもの

テンプレート

社内報の元になるテンプレート。テンプレートには[名前]といった風に、各メンバーの本文挿入箇所が指定されている。

メンバー表

メンバーの名前、メールアドレスが定義されている。メールアドレスの形式は名前@ドメイン。

各メンバーの原稿

ファイル名が年月＋名前.txtというフォーマットのテキストファイルが毎月送付されてくる。

機能

社内報の生成
1. ディレクトリに、メンバーから提出された原稿を全て格納する。
2. １のディレクトリを指定する。
3. アプリの作成ボタンを押下すると、テンプレートの内容を元に社内報が生成される

私「こんなところかな？…………。」

なんこつ「急に黙ってどうしたの？」

私「いや……なんかさ、このアプリを初めて見た時……一年半くらい前なんだけどね？なんかめちゃくちゃ凄いアプリだな〜って思って、こんなの私に作れるわけない！って思ってたんだよ。私も結構へっぽこだったからさ……。」

私「でも、こうやって見ると そんなに難しいことはやってないんだなって感じて……あ、勿論機能が単純とかそういう話じゃなくて、ロジックをイメージしたときに凄いシンプルだなって思って。」

なんこつ「（静かに微笑む）」

私「ちょっとその顔やめてもらっていいです？？？」

なんこつ「いやぁ……なんか……ねぇ。いいねぇ。」

私「時を戻そう」

なんこつ「最近のブームをしれっと混ぜ込んで来ないで」

私「今回はとりあえず、現行のアプリで最低限必要なところの設計・実装を行うって話だったから、現行の内容で十分なんじゃないかなって思うんだよね。次回で欲しい機能を追加する、って話をかき揚げ先輩としてるしね。」

なんこつ「じゃあ、一旦内容を纏めて先輩に見ていただきましょうよ」

私「そだね、もしかしたら何かアドバイスいただけるかも！」

〜〜〜そして先輩との打ち合わせ後にできた今回の仕様が以下〜〜〜

社内報作成アプリ

問題

テンプレートに原稿を挿入し、社内報を作成する。

社内報の元データ

app/fixture/template.txt ・・・社内報のテンプレート。
app/fixture/member_list.csv ・・・各メンバーの名前とメールアドレスが記載されている。
app/fixture/manuscripts ・・・ 年月 毎にディレクトリを作成し、直下にその月の各メンバーの原稿を格納する。

要求仕様

引数には文字列で 年月 を渡す（例: 201902）
引数に該当するディレクトリ内にある原稿を取得し、テンプレートに当て嵌めて社内報を作成する。社内報は文字列として返す。
指定したディレクトリが存在しない場合は例外を返す
テンプレートの [user_name] の部分は ・ユーザ名 に変換し、その次の行から本文を記載する。
原稿と原稿の間には必ず １行分の改行を含む こと
原稿の末尾に不要な改行が含まれていたら取り除く
原稿が未提出の社員については本文に 未提出 と記載する

例） 202002を引数に指定した場合、以下のように出力される

・ラテ太郎
お疲れ様です、ラテ太郎です。
サンプル
サンプル
サンプル

・ラテ子
お疲れ様です、ラテ子です。
サンプル
サンプル
サンプル
サンプル

・沖漬け先輩
サンプル
サンプル
サンプル
サンプル
サンプルサンプルサンプルサンプルサンプルサンプルサンプル

・ラテ二郎
未提出

テンプレート

db/template.txt

社内報

[rate_taro]

[rate_ko]

[okiduke_senpai]

[rate_jiro]

db/member_list.csv

ラテ太郎,rate_taro@sample.com
ラテ子,rate_ko@sample.com
沖漬け先輩,okiduke_senpai@sample.com
ラテ二郎,rate_jiro@sample.com

app/internal_newsletter.rb

class InternalNewsletter
  def generate(datestr)
    ''
  end
end

※ディレクトリが存在しないとき、既存アプリではエラーにならずに処理を完了してしまっていたため、
例外を起こすことでエラーがわかるようにする、という仕様が増えました。

作成した仕様を元に実際に解いていく

私「よし！実際に作っていくよ〜〜〜！」

私「まずは私のイメージを#generateにメモしていこう」

app/internal_newsletter.rb

require 'csv'

class InternalNewsletter
  def initialize
   # テンプレートとメンバー表を読み込む
  end

  def generate(datestr)
    # 読み込んだテンプレートを一行ずつ取得し、配列を作成する
    # 読み込んだ行が[名前]にマッチする場合、[名前]を ・名前\n本文 に置換する
    # 最後に配列を\nで結合する
  end
end

私「こうやって書くとやりたいことってほんとシンプルだよね〜。次、具体的にコードをかけそうなところを埋めていく。」

app/internal_newsletter.rb

require 'csv'

class InternalNewsletter
  def initialize
    @template = File.open('db/template.txt').readlines(chomp: true)
    @member_list = CSV.read('db/member_list.csv')
  end

  def generate(datestr)
    @template.each_with_object([]) do |row, array|
      # 読み込んだ行が[名前]にマッチする場合、[名前]を ・名前\n本文 に置換する
    end.join("\n")
  end
end

なんこつ「凄いそれっぽいじゃない。」

私「いやいや、問題はここからなんだよね……」

私「テンプレートを一行ずつ読み込んでいくと、rowに入り得る内容は以下の3パターンある。」

見出し(社内報など)
改行のみ
[名前]（置換対象）

私「今回でいうと、3の場合だけ置換対象にしたい。それ以外の場合は手を加えたくない。」

私「だから、3以外は早期リターンでスルーしたいから、次はそこを作ろう」

app/internal_newsletter.rb

 require 'csv'

 class InternalNewsletter
   def initialize
     @template = File.open('db/template.txt').readlines(chomp: true)
     @member_list = CSV.read('db/member_list.csv')
   end

   def generate(datestr)
     @template.map do |row|
+      member_name_en = row[/\[(.*?)\]/, 1]
+      next row if member_name_en.nil?

       # 読み込んだ行が[名前]にマッチする場合、[名前]を ・名前\n本文 に置換する
     end.join("\n")
   end
 end

なんこつ「なにそのmember_name_enに代入してるやつ！」

私「ふっふっふ……やばいでしょやばいでしょやばいでしょ」

私「私は自称正規表現ザコなので、今回は調査を頑張りました。」

私「知ってるかもしれないけど、これはstr[]メソッドってやつでね」

私「第２引数に渡す値で振る舞いが変わるんですよ。」

私「0の時は第2引数無しの場合と一緒の振る舞い。つまり今回だと [名前]が返ってくる。」

私「0以外の場合は、第一引数に渡した正規表現の（第２引数に渡した値）番目の括弧にマッチする最初の部分文字列が返ってくるのね。」

私「今回だと1番目の括弧、つまり(.*?)にマッチした文字列が返ってくる。括弧の中には[]が含まれていないでしょ？だから、名前だけが返ってくる。超かっこよくない？」

なんこつ「なるほどね……これ、結構使えそうな気がするわ。例えば一つの文字列から、３箇所部分的に取得したい内容がある場合、正規表現でそれぞれを括弧で囲っておくと、第2引数の値を変えるだけでそれぞれが取得できるってことだものね」

私「そういうこと！結構便利な気がする！」

[1] pry(main)> '[aaa] [bbb] [ccc]'[/\[(.*?)\]\s\[(.*?)\]\s\[(.*?)\]/, 0]
=> "[aaa] [bbb] [ccc]"

[2] pry(main)> '[aaa] [bbb] [ccc]'[/\[(.*?)\]\s\[(.*?)\]\s\[(.*?)\]/, 1]
=> "aaa"

[3] pry(main)> '[aaa] [bbb] [ccc]'[/\[(.*?)\]\s\[(.*?)\]\s\[(.*?)\]/, 2]
=> "bbb"

[4] pry(main)> '[aaa] [bbb] [ccc]'[/\[(.*?)\]\s\[(.*?)\]\s\[(.*?)\]/, 3]
=> "ccc"

※参考
[Ruby]特定の文字列の抽出
 Ruby 2.7.0 リファレンスマニュアル

私「と、これで取得した行が[名前]以外の場合はスルーできるようになった！」

私「次、実際に原稿を挿入していく処理を作るよ！」

app/internal_newsletter.rb

 require 'csv'

 class InternalNewsletter
   def initialize
     @template = File.open('db/template.txt').readlines(chomp: true)
     @member_list = CSV.read('db/member_list.csv')
   end

   def generate(datestr)
     @template.map do |row|
       member_name_en = row[/\[(.*?)\]/, 1]
       next row if member_name_en.nil?

+      "・#{@member_list.find { |arr| arr[1].match(/\A#{member_name_en}/) }[0]}\n#{load_manuscript(datestr, member_name_en)}"
     end.join("\n")
   end

+  private

+  def load_manuscript(datestr, member_name_en)
+    file_path = "db/manuscripts/#{datestr}/#{datestr}#{member_name_en}.txt"
+    return '未提出' unless File.exist?(file_path)

+    File.open(file_path).read.strip
+  end
 end

私「[名前]の場合、・名前\n本文を返す。けど、もしその人が原稿を提出していなかったら、本文は未提出という文言を出したい。」

私「条件によって、本文の内容が変わるから、そこをよしなにできるメソッドとして#load_manuscriptをつくったよ。」

なんこつ「へぇ……なかなかいいんじゃないの」

私「だよねだよねだよね〜？ちょっとこれで動かしてみようかな！」

社内報

・ラテ太郎
吾輩は猫である。
名前はまだ無い。
どこで生れたかとんと見当がつかぬ。
何でも薄暗いじめじめした所でニャー

・ラテ子
恥の多い生涯を送って来ました。
自分には、人間の生活というものが、見当つかないのです。
自分は東北の田舎に生れましたので、汽車をはじめて見たのは、
よほど大きくなってからでした。

・沖漬け先輩
木曾路はすべて山の中である。
あるところは岨づたいに行く崖の道であり、
あるところは数十間の深さに臨む木曾川の岸であり、
あるところは山の尾をめぐる谷の入り口である。

・ラテ二郎
未提出

私「待って」

私「最後の改行が足りないよ〜〜〜！？！？！？」

私「ぴえんぴえんのぴえんだわ」

私「ぴえんすぎる」

私「そっか……joinでくっつけてるから最後の改行がくっつかないのか……」

私「う〜〜〜〜〜ん……リファクタしながら考えるか……。」

〜〜〜リファクタリング後〜〜〜

require 'csv'

class InternalNewsletter
  def initialize
    @template = File.open('db/template.txt').readlines(chomp: true)
    @member_list = CSV.read('db/member_list.csv')
  end

  def generate(datestr)
    @template.each_with_object([]) do |row, array|
      member_name_en = row[/\[(.*?)\]/, 1]
      next array << row if member_name_en.nil?

      array << "・#{@member_list.find { |arr| arr[1].match(/\A#{member_name_en}/) }[0]}"
      array << load_manuscript(datestr, member_name_en)
    end.join("\n") + "\n\n"
  end

  private

  def load_manuscript(datestr, member_name_en)
    file_path = "db/manuscripts/#{datestr}/#{datestr}#{member_name_en}.txt"
    return '未提出' unless File.exist?(file_path)

    File.open(file_path).read.strip
  end
end

なんこつ「ちょっと……」

私「完全敗北しました……文字列結合で\n\nとかはちゃめちゃにダサダサなのわかってるんですけど……勝てなかった……ごめんなさい……！！！」

私「代わりと言ってはなんだけど、mapの中で・名前\n本文みたいに改行が出てくるのが嫌だったから、改行コードはmapで配列を作り終わった後に一括で付けるようにしました。」

私「いや〜〜でもやっぱり一番最後で文字列結合した改行×２がやっぱり嫌だ……許せれん……。どうしたらいいんだろうね……」

レビューをいただく

かき揚げ先輩「じゃあレビューしようか」

私「お願いします！」

かき揚げ先輩「まず残念なお知らせです。」

私「ひゃい」

かき揚げ先輩「ディレクトリが存在しない時ってどうするんだっけ？」

私「」

私「」

私「あ！？！？！？！！？？！？！？！？！？！？！」

私「例外！！！！！！！！！！！！」

かき揚げ先輩「漏れてました」

私「ああああああああああ」

私「今回割と自信あったのに！！！！！！！！！！」

私「穴があったら……ってやつですウワ……しくった」

かき揚げ先輩「次はコードね」

かき揚げ先輩「まずね、[名前]の名前だけ抜き出すやつ」

かき揚げ先輩「matchを使わないあの書き方は初めて見た。」

かき揚げ先輩「短く書けていいと思いました」

私「（やった〜〜〜〜〜〜）」

かき揚げ先輩「次、テンプレートを元にする処理でeach_with_objectを使ってたよね？」

私「はい」

かき揚げ先輩「あそこを一行で書けてたらmapでもよかったんだよね」

私「そうなんです……」

かき揚げ先輩「今回で言うとフォーマットがきっちり決まっていたから、そこを共通化できたらよかったかな」

かき揚げ先輩「あとで俺の書いたコード見せるけど、こういう時こそヒアドキュメントを使うといいよね。ヒアドキュメントの場合、文末に改行が必ず付くから。」

私「！！！！！！」

私「な、なるほど……確かに。しかもヒアドキュメントなら、実際に表示される文章とほぼ同じ形式で書くから可読性も高いですよね」

かき揚げ先輩「そういうこと。ハードコーディングな部分をメソッドに切り出すこともできるから、そういう意味でもより可読性は上がるね。

かき揚げ先輩「次は実際に俺の書いたコードを見てもらおうかな。」

require 'csv'

class InternalNewsletter
  MEMBER_LIST = CSV.read('db/member_list.csv')
  NEWSLETTER = File.read('db/template.txt')
  NOT_SUBMITTED = '未提出'.freeze

  def initialize(datestr)
    @datestr = datestr
    @dirpath = "db/manuscripts/#{@datestr}"
  end

  def generate
    raise ArgumentError unless Dir.exist?(@dirpath)

    MEMBER_LIST.inject(NEWSLETTER, &method(:newsletter)) + "\n"
  end

  private

  def newsletter(text, member)
    name, keyword = split_name_keyword(member)
    fullpath = "#{@dirpath}/#{@datestr}#{keyword}.txt"
    body = File.exist?(fullpath) ? File.read(fullpath).strip : NOT_SUBMITTED

    text.gsub(
      /\[#{keyword}\]/,
      <<~NEWSLETTER.chomp
        ・#{name}
        #{body}
      NEWSLETTER
    )
  end

  def split_name_keyword(member)
    name, email = member
    _, keyword = email.match(/\A(\w+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\z/).to_a
    [name, keyword]
  end
end

私「はぁ〜〜〜〜なるほど……」

かき揚げ先輩「俺の処理だと早期リターンはなくて、該当箇所を置き換えていくだけ。」

かき揚げ先輩「次のステップとして、私ちゃんに意識して欲しいのは共通部分と異なる部分をそれぞれ見つけて、共通化していくことだね」

かき揚げ先輩「共通化することによって、記述を減らすことができるし、コードの量が少ないって事は読む量が少ないってこと。共通化して、総コード数を減らせるようになるとより良くコーディングができるようになると思うので、意識してみてください」

私「分かりました！ありがとうございます！」

最後に

自分なりにより良い書き方を考えて書いたつもりだったのですが、ヒアドキュメントは盲点でした……。先輩のコードを拝見した際に思わず唸ったほどです。
あと例外の話をすっかり忘れていたといううっかりは本当に悔やんでも悔やみきれませんでした。こういううっかりを減らしていくためにも、最初のメモ書きを行った時点で仕様を満たせているか確認した方がいいですね。

一方で、正規表現周りで先輩に「知らなかった」と言っていただけた事、また「ロジック自体はそんなに難しくないな」と思えた事は私的に成長を感じられる嬉しみポイントでした。

上手く出来なかったところは改善しつつ、よかった所も噛み締めながら、この調子で引き続き頑張りたいと思います！