社長が書いたクソコードたち

社長が書いたクソコードたち 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 => [email protected](:conditions => ["deleted_at IS NULL"]) } end def tab_parks { :label => ‘公園', :component => { :controller => '/view/details/city/parks', :action => 'index', :id => @city.id }, :index => [email protected](: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 などなど

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

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

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

楽しもう！

社長が書いたクソコードたち

社長が書いたクソコードたち

Yuichi Takeuchi

More Decks by Yuichi Takeuchi

Other Decks in Programming

Featured

Transcript