社長が書いた クソコードたち RAILS DEVELOPERS MEETUP 2017飛び入り @TAKEYUWEB

タケユーさんって呼んでください！ • Railsの受託会社やってます！ • Rails書いてます！ • Railsアドベントカレンダー＆Rubyアドベントカレンダー書きました！

通勤したくないので完全リモートの会社社長やってます

タケユー・ウェブ株式会社は コードをよくしていきたい RAILSエンジニアを募集しています！ • Railsエンジニア 一緒に開発をしてくれる方 フリーランス、社員問わず募集中 • 完全リモート前提 自宅に居ながらにしてLGTMって 言ったり言われたりする仕事 地方在住でもOK • 完全フレックス＆短時間OK 週3日から好きな日に ご興味を持っていただけましたら、ぜひお声がけください！ https://www.wantedly.com/projects/99879

さいたま.RB（仮） • ２０１８年１月キックオフ予定 • 大宮駅前 • コワーキングスペース７F（貸会議室６F) 埼玉の人やりましょう！ https://saitamarb.connpass.com/

閑話休題

この間、 １０年前に書いたRAILSアプリのメンテしました • Ruby 1.8.6 ～2.0 • Rails 1.1.6 ～2.2

…

うわっ！私のコードクソすぎ！？

今日はそこで見たクソコードの中で 一目でクソさがわかる ライトなクソのごく一部を 紹介します！

謎のGOOGLE MAPSラッパー（RUBY) • よくわからないActionとかいうクラスを積み上げてる。これがControllerに延々と書いてあっ た • Google Maps JavaScript APIのラッパーのJavaScriptを吐いてた。独自ラッパーの独自ラッ パー。 • 独自ラッパーやめろ。「これ、便利じゃね？」じゃねぇ。設計間違いなくヤバイ map = Mapper::Updater.new map.actions << Mapper::Action::CreateMarker.new(create_marker_options(member)) map.actions << Mapper::Action::SetCenter.new(:pan => true, :lat => member.lat, :lng => member.lng)

INCLUDEするのにEXTENDEDな名前 • そもそもおかしな名前だけど『extend』して使うのかな？→includeしてる • prepend_before_filter でコールバックが順序に依存してるのに順序が混乱 • しかもparams書き換えてるー • いろいろ言いたいことあるけどとりあえず名前ちょっと考えようか module ExtendedController def self.included(receiver) receiver.send(:prepend_before_filter, :set_controller_name_on_component) receiver.send(:after_filter, :fix_unicode_for_safari) end class ApplicationController < ActionController::Base include ExtendedController

謎の継承CONTROLLER • どんなアクションがあるのかすらわからない • さらにこの親コントローラーもいろいろ継承してる、includeもりだくさん • 実体がどこにあるのかもわからない • なぜか lib 以下においててincludeしてた • actionはそのControllerに書くのと、変にMix-in/継承使いたがるのやめようか class View::Details::Member::HogeController < View::Details::Member::Base include Search::Store scoped_access Spot end

謎の継承CONTROLLER（2） • あるページに表示するタ ブの中身 • 対応するControllerとか の情報がなぜか別の Controllerのメソッドに • 何やってるのかわからん • 本当に読めない • 本当に読めない（２回 言った） def tab_stores { :label => ‘お店', :component => { :controller => '/view/details/city/stores`, :action => 'index', :id => @city.id }, :index => -@city.stores.count(:conditions => ["deleted_at IS NULL"]) } end def tab_parks { :label => ‘公園', :component => { :controller => '/view/details/city/parks', :action => 'index', :id => @city.id }, :index => -@city.parks.count(:conditions => ["deleted_at IS NULL"]) } end

謎の独自RENDERメソッド • 何が起こるかわからないし、たぶん全然シンプルじゃない <%= simple_render_component '/view/search/map', 'index', params %>

アンドキュメント・メタプログラミング • ？？？ [:save, :save!, :update_attribute].each{|attr| define_method(attr){}} def method_missing(symbol, *param) if(symbol.to_s =~ /(.*)_before_type_cast$/) send($1) end end

RAILSデフォルトのメソッドの挙動変更 • バグっても気づけない • 本当やめてほしい def find(*args) # 標準の find(:first, :conditions => …) を拡張して挙動が変わるコード

なんかすごい多機能のSEARCHメソッド • どんな検索ができるか（オプションがあるか）も一目でわからない • 全部使ってるかも定かではない • テストもないし怖くて触れない class << self def search(conditions = { }) conditions = { }.merge(conditions) conditions[:order] ||= ‘count.desc’ conditions[:limit] = conditions[:limit] ? conditions[:limit].to_i : 20 conditions[:page] = conditions[:page] ? conditions[:page].to_i : 1 finder_options = { # 以下300行ぐらいconditionsをみて複雑なクエリをこねくり回すコード

読めないAREL • 当時すごく頑張って書いた（これは３年ぐらい前だから割と最近 • 素直にSQL書け memberships_table = User::Membership.arel_table plan_free = User::Plan.find_by(free: true) if plan_free.present? free_membership_table = memberships_table .project(Arel.sql('*')) .where(memberships_table[:plan_id].eq(plan_free.id). and(memberships_table[:paid_at].eq(nil))) .as('free_membership_table' free_membership_join_conds = users_table. join(free_membership_table, Arel::Nodes::InnerJoin). on(users_table[:id].eq(free_membership_table[:user_id])). join_sources

まだまだある。

ぼくのコードはクソにまみれている。

クソコードを 理解できるようになれたともいえる

クソコードを書こうとして 書いたわけじゃない。

結果として クソコード だったけれど

あがいていただけだ

デザインパターン 抽象化 DRY などなど

正しく使えないと ただのクソコード

今日書いた このイケてるコードも、 明日の僕には クソコードかもしれない

あの時書いた よくないコードを理解すれば、 今日はもっとうまく 書けるようになれる

楽しもう！