Add coop command #134
Add coop command #134
Conversation
変更点
コードレビューで頂いたアドバイスを基に，コードを修正しました |
|
452a3ef
to
e5774f8
Compare
変更点
いただいたコメントを基に，コードを修正しました |
lib/swimmy/command/coop.rb
Outdated
| module Swimmy | ||
| module Command | ||
| class Coop < Swimmy::Command::Base | ||
| class ShopInfoFormatter |
There was a problem hiding this comment.
現段階では class にまでしないで ShopInfo の to_s メソッドにするのがいいでしょう．
lib/swimmy/resource/coop_info.rb
Outdated
| end | ||
|
|
||
| def open? | ||
| if @state == "OPEN" |
There was a problem hiding this comment.
1行 @State == "OPEN" だけで十分です
lib/swimmy/resource/coop_info.rb
Outdated
| end | ||
|
|
||
| def name | ||
| s = @f_name.chomp |
There was a problem hiding this comment.
chomp や strip は，initialize で代入するときにしておいたおうがいいです
lib/swimmy/resource/coop_info.rb
Outdated
| @@ -0,0 +1,29 @@ | |||
| module Swimmy | |||
| module Resource | |||
| class ShopInfo | |||
There was a problem hiding this comment.
ファイル名 coop_info と クラス名 ShopInfo が対応が取れていません．CoopShop でしょうか．
lib/swimmy/service/coop_service.rb
Outdated
| text.lines(chomp: true) do |char| | ||
| if d >4 | ||
| shops << Swimmy::Resource::ShopInfo.new(f_name,shopdata[0],shopdata[1],shopdata[2],shopdata[3]) | ||
| d = 1 |
There was a problem hiding this comment.
d とは，何で 4とは何でしょう．i もよく分かりません．
shopdata のような配列を使うのではなくて，それぞれが名前を識別できるような個別の変数を用意して，それを埋めていくような構造にしたほうがいいです．
lib/swimmy/service/coop_service.rb
Outdated
There was a problem hiding this comment.
全体的に何をしようとしているのか理解し辛いです．
スクレイピングしようとしている HTML 全体がどういう構造になっているのかの概略や，
next_element を取り出す意味などをコメントで説明するといいです．
lib/swimmy/service/coop_service.rb
Outdated
| text.slice!(0) | ||
| text.lines(chomp: true) do |char| | ||
| if d >4 | ||
| shops << Swimmy::Resource::ShopInfo.new(f_name,shopdata[0],shopdata[1],shopdata[2],shopdata[3]) |
lib/swimmy/resource/coop_info.rb
Outdated
| attr_accessor :f_name,:state,:n_name,:time,:ex | ||
|
|
||
| def initialize(f_name,state,n_name,time,ex) | ||
| @f_name, @state, @n_name ,@time ,@ex = f_name, state, n_name, time, ex |
There was a problem hiding this comment.
これは，オプショナルですが，
time は，相手の文字列をそのまま使うのではなくて，2つの時刻(数値)に置き換えて保存したほうがいいでしょう．start_time, end_time のような．
そのほうが，フォーマッタや比較が楽になります．
lib/swimmy/resource/coop_info.rb
Outdated
| end | ||
|
|
||
| def open? | ||
| if @state == "OPEN" |
There was a problem hiding this comment.
オプショナルですが，本来は，OPEN かどうかは，時刻から判断すべきで，open?(aTime = Time.now) のようなインタフェースになると思います．
c313b6c
to
672bb6e
Compare
変更点
いただいたコメントを基に，コードを修正しました |
概要
coop コマンドの追加
コマンドの概要
機能
岡山大学生協の営業中の店舗および営業時間を表示する．
使用方法
swimmy coop openもしくはswimmy coop timeと入力する．特徴
岡山大学生協webサイトをスクレイピングし，取得したデータを整理し表示する．