code-your-ruby ある一定のルールで運行している電車の時刻表を算出する機能

このPRについて

こちらの仕様の処理を実装しました

実行結果

cd user/h5y1m141
ruby ./main.rb

上記実行結果は以下のように表示されます

各駅停車の時刻表
["11:10", "11:14", "11:18", "11:23", "11:27", "11:31", "11:35", "11:39", "11:43", "11:47", "11:51", "11:55", "11:59", "12:03", "12:07", "12:11", "12:15", "12:19", "12:23", "12:27", "12:31", "12:34"]
急行の時刻表
["11:15", "11:18", "11:20", "11:22", "11:24", "11:26", "11:28", "11:30", "11:33", "11:35", "11:37", "11:39", "11:42", "11:44", "11:46", "11:48", "11:51", "11:54", "11:57", "12:00", "12:03", "12:05"]

テストについて

MiniTestは使ったことがないのでこんな書き方で良いのかどうか悩みながらも一応

各駅停車
急行停車

の時刻表算出時のベースになりそうなメソッドに対して網羅しました。

参考

コード読む上で参考情報を記載しておきます

それぞれのクラスの役割

各駅停車の処理を例にシーケンス図まとめました。

timetable

実装上の工夫点

仕様理解が難しい（特に各駅停車が急行通過待ちをするあたり）というのが最初の印象だったので、

  def detect_stations_for_waiting_express
    # 後続の急行がある場合
    if [10, 30, 50].include?(@current.min)
      ['SS3']
    else
      ['SS9']
    end
  end

の所でそこをうまく吸収できるようにあらかじめ実装してみました

Jan 10 '18 07:01 h5y1m141

プルリクありがとうございます！見てみます。

僕以外の方からの意見も集まるよう、現在このリポジトリを宣伝中ですので、気長にお待ちいただければと思います。

Jan 12 '18 05:01 chooyan-eng

現在このリポジトリを宣伝中ですので、気長にお待ちいただければと思います。

全然大丈夫ですよ～

私も身の回りで、ちょっとづつこのリポジトリ宣伝してるので、ちょっとづつ興味ある人が増えていけば良いですね 😄

Jan 12 '18 08:01 h5y1m141

cc: @chooyan-eng

試しにレビューしてみましたけど、いまいちピンと来なかったです。これがもし職場の OJT だったらデータモデリングから見直して解答例というか見本となるようなコードを示したりするんですけどそこまでするのも正直しんどいよなぁという感じです。

例えば「このメソッド名はこういう理由で良くないですよ」ってコメントしても、その背景となる DSL の考え方だったりとか、責務についての考え方なりの諸々の解説をしないと結局伝わらないと思うんですけど GitHub のコードレビューでそこまでするのもしんどいですしねえ

Mar 01 '18 10:03 ttanimichi

@ttanimichi レビューありがとうございます！

試しにレビューしてみましたけど、いまいちピンと来なかったです。

について、お伺いしたいのですが、ピンと来ない理由として

このリポジトリにおいて、レビューする時にどのスタンスで取り組むべきかピンと来ない
このPRの実装＆設計意図がよくわからない点が多くピンと来ない
上記の1.と2.の両方の意味
その他

のどれになりそうなのでしょうか？

PR出してから、2ヶ月ほど何もフィードバックなかったので、レビュアーとして関わろうとした時にどういうスタンスで望めばよいのか各自考え方異なりそうで結果

責務についての考え方なりの諸々の解説をしないと結局伝わらないと思うんですけど GitHub のコードレビューでそこまでするのもしんどいですしねえ

というのが出てきて、結局コードレビューするのがツラいから今までの状況につながるのかなとふと感じたので。

Mar 01 '18 21:03 h5y1m141

ピンと来ない理由

1 に近いですかね。どこまで深くレビューすべきなのかなっていうのがふわっとしてて。あまり表面的なレビューだけしても価値が薄いでしょうし

結局コードレビューするのがツラいから今までの状況につながるのかな

FizzBuzz の PR をしてる人が多いですが、FizzBuzz のコードなんかを見ても「ふーん」とか「へえ」くらいの感想しか出てこなくてレビューできる気がしなかったので、この PR のレビューをすることにしました。

Mar 02 '18 04:03 ttanimichi

@ttanimichi @hoyamada どのレベルでレビューするかがふわっとしている点、詳しくじっくりレビューするほど労力かけられない、且つGitHub上のやりとりでは限界がある点、、課題ですね。。。ご意見ありがとうございます！

このプルリク上でやりとりが続くとコードに対するコメントと混ざってしまうため、issue #10 を作りました。もしよろしければこちらで議論させていただければと思います。

Mar 02 '18 04:03 chooyan-eng