プログラミング学習メモ

RubyとRuby on Rails等

Rakeタスクをレビューしてもらった

前回の記事でまとめた情報を元にRakeタスクを作りました。

起動すると、はてなブックマークのランキングをスクレイピングし、タイトルを出力し、記事を選択してブラウザで開くという物です

github.com

これに対し

  • nokogiriは外部ライブラリなのでGemfileで管理
  • open-uriでエラーが出た際の例外処理がない
  • 処理を分割して実装する
  • STDIN.gets.chompはCLIのツールを用いる?
  • classを使う

というissueをもらったので

それを踏まえて書き直したのが、こちら

github.com

これをビデオチャットでレビューしてもらいました。 個人的にはそこそこできたと思っていたのですが、かなり深刻な問題点を指摘・修正してもらいました。

レビューまとめ

実際はもっと細かく指摘されましたが、自分なりにまとめてみました。振り返られるのがpull requestのコードと記憶のみなので無意識に素通りしている所はあるかも知れません。

クラスの区分を考える

私が作ったタスクはclassがRankingScrapingしかなく、スクレイピング・タイトルとURL格納・タイトル出力・記事の選択とブラウザ移行が一つのクラスに記述されていました。
このクラス名はScraperの様な名詞に、行う処理はスクレイピングと出力のみにし、要素(タイトル・url)の格納とコレクション等は別のクラスを作った方が良いです。

紐付いた値はまとめて管理する

タイトルはtitle_ary、URLはurl_aryという配列に格納していましたが、これらは本来セットとなる要素です。取り回し辛い上に、そもそもRubyは配列の順番を保証していません。
これらには新しいクラスを作り、そのインスタンスインスタンス変数にタイトルとURLとランキングを格納という方法が良いです。
これらのコレクションクラスも必要です。

固定の値はクラス内部に隠す

今回はスクレイピングしたいURLやxpathcssセレクタを外部からもらう様なことはせず常に固定です。にも関わらず、固定のURLやxpathを変数としてRakefike内に記述していました。これらは値を扱うクラスに定数として置くのが好ましいです。

メソッド名は簡潔に

get_elementやchoice_articleなど冗長なメソッド名が多いです。elementやselectなど可能なら一語で書くのがrubyの慣習らしいです。rubyの慣習にそったコーディングか判断してくれるRubocopというgemもあるので今度使ってみようと思います。

例外処理は外側に書く

クラスのメソッド内で例外処理を書いていました。結果的に握り潰してしまうことになるので、実行ファイルであるRakefileに記述しました。また、例外処理でプログラムの実行を終了する場合のexitの戻り値はエラーを意味する1にします。エラーでない場合は0ですが、それがデフォルトなので引数なしでも正常終了扱いとなります。

既存の機能を活用する

先ほど言及したコレクションクラスを定義する時にはEnumerableを使います。 Enumerableはmix-inしeachを定義するだけでコレクションクラスが簡単に作れる、Arrayクラスもincludeしている繰り返しの汎用モジュールです。rubyはこういった便利な汎用モジュールがあるのでクラス作成にそれを使わない手はないです。

感想

基本的なプログラミングと設計の力がないこと、クラスを使ってオブジェクト指向的にプログラミングする難しさを再認識しました。クラスに関しては数をこなさないと身につかないと言っていたので、これからはもっとコードを書いて少しづつでも身に付けられたらと思っています。