既存のコードを変更する時の心得 ～ どこを見ればいいかの勘所 ～

既存のコードを読み解くにあたって注意して読むべきポイント、読み進め方などをご紹介しています。対応する演習用の資料も含めたリポジトリが https://gitlab.com/clear-code/presentation-lets-modify-existing-code にあります。

Transcript

1. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 既存のコードを変更する時 の心得 　　 〜どこを見ればいいかの勘所〜 結城洋志 株式会社クリアコード
2. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 開発でよくある場面 必要な機能が無いから追加したい ✓ プラグインの機能を本体に統合し たい ✓ 不具合を直したい ✓ など
3. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 よく考えてみよう ゼロから開発する 場合以外はすべて 既存のコードの 変更である
4. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 つまり 変更を制する者が 開発を制する！
5. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 既存のコードを変更する時の流 れ 変更対象のコードを把握する 1. 変更の計画を立てる 2. 自動テストを書く 3. 実際に変更を行う 4.
6. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 実際には 変更対象のコードを把握する 1. 変更する 変更の計画を立てる a. 自動テストを書く b. 実際に変更を行う c. 2.
7. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 つまり 変更対象のコード を把握する事 のウェイトが非常 に大きい。
8. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 ありがちな場面 「とりあえずコードを読んでおいて」 ✓ 「とりあえず資料を読んでおいて」 ✓
9. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 読んだけど……？ 漫然とコードや資料を 読んでおけばいい、というわけ ではない ✓ 「何を読むか」ではなく、 「何を読み取るか」が重要 ✓
10. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 目的を意識して読んでみよう 「ある情報」を 読み取るために 読もう
11. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 ある情報、とは？ 周囲のコードに 馴染むコード を書くために 必要な情報
12. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 理想的な仕上がりの「変更」 現在は、この機能がない・こういう 問題がある ✓ 機能があったとしたら・問題がなか ったとしたら、当然こう書かれてい るはずだろう （元々作った人が実装していたら、 当然こう実装されていただろう） ✓
13. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 それをするためには？ 元作者になりきる ✓ 前提知識を共有した チームの一員になる ✓ ……ための情報が必要
14. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 やらないとどうなる？ ある関数はsnake_case、 別の関数はcamelCase バラバラな名前付け ✓ ✓ 同様の関数が各モジュールにある 車輪の再発明 ✓ ✓ 「とりあえず動くように」で 無理矢理繋げたモジュール スパゲッティコード化 ✓ ✓
15. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 カオス化 カオスなコードは 変更しにくい
16. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 馴染むコードにするために コーディングスタイルを合わせる ✓ 設計方針を合わせる ✓ 背景事情に合わせる ✓
17. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 つまり、読み取るべき情報は コーディングスタイル ✓ 設計方針 ✓ 背景事情 ✓ どういう所を見ればそれらが分かる？
18. 質問？
20. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 読み取りたい情報 コーディングスタイル ✓ 設計方針 ✓ 背景事情 ✓
21. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 コーディングスタイル コード全体 ✓ スタイルガイド、コーディング規約 （ある場合） ✓
22. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 コーディングスタイルの例 命名規則 ✓ スペースの空け方、詰め方 ✓ 改行の入れ方 ✓ 「この機能は使わない」 「この書き方はしない」 ✓
23. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 命名規則1 ClassName （Pascal Case / Upper Camel Case） ✓ variableName （Camel Case / Lower Camel Case） ✓ variable_name （Snake Case） ✓ file-name （Kebab Case / Chain Case） ✓ CONSTANT_NAME （すべて大文 字） ✓
24. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 命名規則2 String#include （原形） ✓ String#includes （三人称単数形） ✓ items（複数形） ✓ itemArray（単数形） ✓
25. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 命名規則3 よく使われるネーミング isMultiple / hasItem ✓ getName / setName ✓ fetchData ✓ valid? / save! (ActiveRecord) ✓
26. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 文字列リテラルの定義の仕方 single = 'シングルクォート？' double = "ダブルクォート？" 使い分けるのか、常にどちらかに統一 するのか
27. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 行末のセミコロンの有無 javascriptでは行末の「;」を省略できる const x = 0 const y = 10 const x = 0; const y = 10;
28. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 いわゆるケツカンマなし items = [ a, b, c ] props = { a: 0, b: true, c: false }
29. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 ケツカンマあり items = [ a, b, c, ] props = { a: 0, b: true, c: false, }
30. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 スペースの空け方、詰め方 if(count==3){...} if (count == 3) { ... } if ( count == 3 ) { ... }
31. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 インデント幅 if count == 3 p 'OK' if count == 3 p 'OK' if count == 3 p 'OK'
32. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 変数はvar？ let？ const？ JavaScriptでは変数の定義の仕方が3 種類ある var b = true; // 広いスコープの変数 let c = ''; // 狭いスコープの変数 const a = 10; // 狭いスコープの変更不能な変数（定数）
33. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 変数はいつ定義する？1 // 先頭で固めて定義 function f() { const a = ... const b = ... ... }
34. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 変数はいつ定義する？2 // 必要になった場面で、可能な限り小さなスコープで定義 function f() { ... for (const prop of object) { ... } ... }
35. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 縦を揃える/揃えない x = 10 y = 20 delta = 300 // 基本的に*単語の頭*を揃える x = 10 y = 20 delta = 300
36. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 改行の入れ方1 if (condition) { ... } if (condition) { ... } if (condition) { ... }
37. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 改行の入れ方2 if (condition) { ... } else { // 同じ行でelse ... } else { // 改行してからelse ... }else{ // 詰めて書くかどうかの違いもある ... }
38. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 ブレースかdo〜endか while condition { do_something do_another } while condition do do_something do_another end
39. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 メソッド呼び出しの丸括弧 p (1..10).inject(0) {|sum, i| sum + i } p((1..10).inject(0) {|sum, i| sum + i }) p (1..10).inject(0) do |sum, i| sum + i end p((1..10).inject(0) do |sum, i| sum + i end) エッジケースで挙動が変わる事もある
40. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 early returnするかどうか def f(condition) if condition # 本来の処理 end end def f(condition) return unless condition # 本来の処理 end
41. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 メソッドは機能的に関係してい る物を集める？ def init ... def getName ... def setName ... def import ... def export ...
42. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 メソッドは名前順で並べる？ def export ... def getName ... def import ... def init ... def setName ...
43. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 後置ifを使うかどうか return if condition if condition return end
44. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 unlessを使うかどうか if !condition ... end unless condition ... end
45. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 「!」か「not」か if !condition ... end if not condition ... end
46. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 「map」か「collect」か results = items.map {|item| ... } results = items.collect {|item| ... }
47. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 「これは使わない」というコンセ ンサス1 便利な機能でも「使わない」 判断をしている場合がある # 三項演算子 count = enabled ? 10 : 1;
48. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 「これは使わない」というコンセ ンサス2 # 比較の左辺に代入不可能な値を置く if true == enabled ... end if true = enabled # typo ... end
49. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 「こう書く事もできる」 色々な書き方ができる≠色々な書き 方をしていい ✓ どの書き方でもいいので、統一する 事が大事 ✓ 迷ったら有名プロジェクトのコーデ ィングスタイルをそのまま採用 ✓
50. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 良いコードと馴染むコードは別 リーダブルコードなどで 「これは冗長」「おすすめしない」 とされているスタイルが 採用されている場合もある ✓ 「馴染むコードを書く事」と 「よりリーダブルなコードに直す事」 は別の作業と考えよう ✓
51. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 整形や検証は自動化しよう JavaScript → ESLint ✓ Ruby → RuboCop ✓ ルールに則っていない部分を 自動訂正したり警告したりする
52. 質問？
54. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 読み取りたい情報 コーディングスタイル ✓ 設計方針 ✓ 背景事情 ✓
55. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 設計方針を読み取る ディレクトリ構成を見る ✓ 自動テストを読む ✓ コードの流れを追う 初期化処理から ✓ 「この機能を調べたい」と 決めた部分から ✓ ✓
56. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 ディレクトリ構成 「この機能を変更するときは このあたりを探せば ファイルが見つかるはず」 という判断に必要な情報
57. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 モジュールの設計上の分類で まとめるのか？ + models | + login.rb | + user.rb + controllers | + login_controller.rb | + user_controller.rb + views + login + user
58. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 アプリケーションとしての機能 単位でまとめるのか？ + model + login | + controller | + view + user + controller + view
59. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 自動テスト TDD（test）形式？ ✓ BDD（spec）形式？ ✓ 形式の違いよりも 中身の方に注目しよう
60. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 テストの「テスト」以外の意義 自動テストは各モジュールの 実際の使い方のサンプル集 である ✓ モジュールの実装を見るより、 テストを見た方がよく分かる ✓
61. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 テストの例 # test/unit/issue_test.rb class IssueTest < ActiveSupport::TestCase ... def test_create_minimal issue = Issue.new(:project_id => 1, :tracker_id => 1, :author_id => 3, :subject => 'test_create') assert issue.save assert_equal issue.tracker.default_status, issue.status assert issue.description.nil? assert_nil issue.estimated_hours end
62. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 テストから読み取れる設計方針 モジュールの分け方 ✓ モジュール同士の連携のさせ方 ✓
63. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 モジュールの分け方の視点1 # ログイン、という行為を主体とするのか success = try_login(user: "example", password: "example") assert success
64. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 モジュールの分け方の視点2 # ユーザー、という人物を主体とするのか success = @user.authenticate(@password) assert success
65. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 モジュールの分け方の視点3 # ログインセッションを主体とするのか session = Session.new(user: @user, password: @password) assert success 同じプロダクトの中でも、Modelの中と Controllerの中など文脈によっても視 点が変わってくる
66. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 モジュールの粒度（責任範囲） 入力した文章を機械学習のために整 形する例 # トークナイズだけを担うモジュールがある words = Tokenizer::Whitespace.call( "this is a list of cool words!") ... # トークンのフィルタリングだけを担うモジュールがある words = TokenFilter::Stopword.call(words)
67. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 責任範囲の分け方 1つのモジュールがどこまでの処理 を担当する？ ✓ この機能はこちらではなくあちらで 実装する、という判断の分かれ目は どこにある？ ✓ という傾向を見よう
68. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 モジュール同士の連携のさせ 方 def test_add_custom_stopword_path temp_stopwords = Tempfile.new('xy', "#{File.dirname(__FILE__) + "/"}") temp_stopwords << "this words fun" temp_stopwords.close temp_stopwords_path = File.dirname(temp_stopwords) temp_stopwords_name = File.basename(temp_stopwords.path) TokenFilter::Stopword.add_custom_stopword_path(temp_stopwords_path) words = Tokenizer::Whitespace.call("this is a list of cool words!") TokenFilter::Stopword.language = temp_stopwords_name # ここまで全て前準備で、以下が本当にテストしたい機能の呼び出し部 words = TokenFilter::Stopword.call(words) assert_equal %w(list cool !), words end
69. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 モジュール同士の連携の様子 の見方 どのような順番で初期化するのか？ ✓ 必要な情報を外部からどうやって 与えるのか？ ✓ を見よう
70. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 コードの流れを追ってみる 初期化処理から・呼び出し元から 順に流れを追う ✓ 目に見えている情報から逆順に処 理の流れを追う ✓
71. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 呼び出し元から順に流れを追う 1 # config/routes.rb Rails.application.routes.draw do root :to => 'welcome#index', :as => 'home' match 'login', :to => 'account#login', :as => 'signin', :via => [:get, :post]
72. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 呼び出し元から順に流れを追う 2 # app/controllers/account_controller.rb class AccountController < ApplicationController ... # Login request and validation def login if request.post? authenticate_user ... ……ということではなく
73. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 真：呼び出し元から順に流れを 追う1 サーバーを起動する方法として分かっ ている手順 $ rails server
74. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 真：呼び出し元から順に流れを 追う2 コマンドの実装を見る # bin/rails APP_PATH = File.expand_path('../../config/application', __FILE__) require_relative '../config/boot' require 'rails/commands'
75. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 真：呼び出し元から順に流れを 追う3 # /home/vagrant/.rbenv/versions/2.5.1/lib/ruby/gems/ # 2.5.0/gems/railties-5.2.1/lib/rails/commands.rb require "rails/command" ... Rails::Command.invoke command, ARGV
76. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 真：呼び出し元から順に流れを 追う4 # /home/vagrant/.rbenv/versions/2.5.1/lib/ruby/gems/ # 2.5.0/gems/railties-5.2.1/lib/rails/command.rb module Rails module Command ... class << self ... def invoke(full_namespace, args = [], **config)
77. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 表示文字列から追う ログの行 ✓ 画面上の表示文字列 ✓ URLに現れた文字列 ✓ これらをヒントにする
78. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 Redmineのログイン処理の流 れを追う Redmineでログインに失敗した時のメ ッセージ ユーザー名もしくはパスワードが無効です
79. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 表示文字列でソースコードを検 索 git grep を使う $ git grep ユーザー名もしくはパスワードが無効です config/locales/ja.yml: notice_account_invalid_credentials: ユーザー名もしくはパスワードが無効です
80. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 「notice_account_invalid_cr edentials」でソースコードを検 索 $ git grep notice_account_invalid_credentials app/controllers/account_controller.rb: flash.now[:error] = l(:notice_account_invalid_credentials) ...
81. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 該当箇所の前後を見る app/controllers/ account_controller.rb をエディタで開いてみる def invalid_credentials logger.warn "Failed login for '#{params[:username]}' from #{request.remote_ip} at #{Time.now.utc}" flash.now[:error] = l(:notice_account_invalid_credentials)
82. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 呼び出し元を逆に辿っていく1 def password_authentication user = User.try_to_login(params[:username], params[:password], false) if user.nil? invalid_credentials
83. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 呼び出し元を逆に辿っていく2 def authenticate_user if Setting.openid? && using_open_id? open_id_authenticate(params[:openid_url]) else password_authentication
84. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 呼び出し元を逆に辿っていく3 def login if request.post? authenticate_user
85. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 裏技 # ruby raise Exception.new("here!") // JavaScript throw new Error('here!') 呼び出し元を調べたい箇所で例外を発 生させると、たいていスタックトレースを 見られる
86. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 設計を「合わせる」という事 今の自分が分かりやすいかどうか ではなく、次にコードを変更する人 が分かりやすいかどうか ✓ 「欲しい物は、一般的な傾向に従っ て探せば見つかる」という状態を維 持する事が大切 ✓
87. 質問？
89. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 読み取りたい情報 コーディングスタイル ✓ 設計方針 ✓ 背景事情 ✓
90. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 背景事情を読み取る ドキュメント ✓ コメント ✓ コミットログ ✓
91. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 ドキュメント （省略）
92. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 コメント 「なぜそうしている のか」が書かれて いることがある
93. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 コメントを読まないと分からな い事情 # app/models/user.rb def self.valid_notification_options(user=nil) # Note that @user.membership.size would fail since AR ignores # :include association option when doing a count if user.nil? || user.memberships.length < 1
94. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 コミットログ git log app/models/issue.rb ✓ git log -p app/models/ issue.rb ✓ などの方法で見られるが、 漫然と読むのは現実的でない
95. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 テーマで辿る1 例：Railsのバージョンアップにあたって どういう作業をしたのか？ git log | grep -i 'rails' -B 4 commit 9947003a1145d66fb5457075908d1b1493763528 Author: Go MAEDA <maeda@farend.jp> Date: Wed Aug 8 13:29:11 2018 +0000 Update Rails to 5.2.1. -- commit ce1c65225037622c10568b3e6955cecce7e80fd9 Author: Jean-Philippe Lang <jp_lang@yahoo.fr> Date: Sat Jun 23 05:13:29 2018 +0000 Upgrade to Rails 5.2.0 (#23630).
96. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 テーマで辿る2 $ git show ce1c65225037622c10568b3e6955cecc e7e80fd9 commit ce1c65225037622c10568b3e6955cecce7e80fd9 Author: Jean-Philippe Lang <jp_lang@yahoo.fr> Date: Sat Jun 23 05:13:29 2018 +0000 Upgrade to Rails 5.2.0 (#23630). git-svn-id: http://svn.redmine.org/redmine/trunk@17410 e93f8b46-1217-0410-a6f0-8f06a7374b81 diff --git a/Gemfile b/Gemfile index 64598a5..98f27f6 100644 --- a/Gemfile +++ b/Gemfile @@ -4,7 +4,7 @@ if Gem::Version.new(Bundler::VERSION) < Gem::Version.new('1.5.0') abort "Redmine requires Bundler 1.5.0 or higher (you're using #{Bundler::VERSION}).\nPlease update with 'ge end -gem "rails", "5.1.6" +gem "rails", "5.2.0" gem "coderay", "~> 1.1.1" gem "request_store", "1.0.5" gem "mime-types", "~> 3.0" diff --git a/app/models/issue.rb b/app/models/issue.rb index 91f53c1..114d962 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -1016,8 +1016,7 @@ class Issue < ActiveRecord::Base # Returns the previous assignee whenever we're before the save
97. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 行ごとの変更履歴から辿る1 git blame app/models/user.rb 027bf938 app/models/user.rb (Jean-Philippe Lang 2007-03-12 17:59:02 +0000 17) 027bf938 app/models/user.rb (Jean-Philippe Lang 2007-03-12 17:59:02 +0000 18) require "digest/sha1" 027bf938 app/models/user.rb (Jean-Philippe Lang 2007-03-12 17:59:02 +0000 19) 77074571 app/models/user.rb (Jean-Philippe Lang 2009-09-12 08:36:46 +0000 20) class User < Principal a4d7a99c app/models/user.rb (Jean-Philippe Lang 2010-12-12 13:19:07 +0000 21) include Redmine::SafeAttributes e86f9711 app/models/user.rb (Toshi MARUYAMA 2011-08-21 01:57:25 +0000 22) 5a1fcf82 app/models/user.rb (Jean-Philippe Lang 2011-11-26 17:37:20 +0000 23) # Different ways of displaying/sortin 79f92a67 app/models/user.rb (Jean-Philippe Lang 2008-01-25 10:31:06 +0000 24) USER_FORMATS = { de0e0f09 app/models/user.rb (Toshi MARUYAMA 2012-10-01 07:07:49 +0000 25) :firstname_lastname => { de0e0f09 app/models/user.rb (Toshi MARUYAMA 2012-10-01 07:07:49 +0000 26) :string => '#{firstname} #{last de0e0f09 app/models/user.rb (Toshi MARUYAMA 2012-10-01 07:07:49 +0000 27) :order => %w(firstname lastname de0e0f09 app/models/user.rb (Toshi MARUYAMA 2012-10-01 07:07:49 +0000 28) :setting_order => 1 de0e0f09 app/models/user.rb (Toshi MARUYAMA 2012-10-01 07:07:49 +0000 29) }, 399223da app/models/user.rb (Jean-Philippe Lang 2012-10-30 08:40:12 +0000 30) :firstname_lastinitial => { 399223da app/models/user.rb (Jean-Philippe Lang 2012-10-30 08:40:12 +0000 31) :string => '#{firstname} #{last 399223da app/models/user.rb (Jean-Philippe Lang 2012-10-30 08:40:12 +0000 32) :order => %w(firstname lastname 399223da app/models/user.rb (Jean-Philippe Lang 2012-10-30 08:40:12 +0000 33) :setting_order => 2 399223da app/models/user.rb (Jean-Philippe Lang 2012-10-30 08:40:12 +0000 34) }, 6e6c6fac app/models/user.rb (Jean-Philippe Lang 2014-01-24 09:23:11 +0000 35) :firstinitial_lastname => { 6e6c6fac app/models/user.rb (Jean-Philippe Lang 2014-01-24 09:23:11 +0000 36) :string => '#{firstname.to_s.gs 6e6c6fac app/models/user.rb (Jean-Philippe Lang 2014-01-24 09:23:11 +0000 37) :order => %w(firstname lastname 6e6c6fac app/models/user.rb (Jean-Philippe Lang 2014-01-24 09:23:11 +0000 38) :setting_order => 2 6e6c6fac app/models/user.rb (Jean-Philippe Lang 2014-01-24 09:23:11 +0000 39) }, de0e0f09 app/models/user.rb (Toshi MARUYAMA 2012-10-01 07:07:49 +0000 40) :firstname => { de0e0f09 app/models/user.rb (Toshi MARUYAMA 2012-10-01 07:07:49 +0000 41) :string => '#{firstname}', de0e0f09 app/models/user.rb (Toshi MARUYAMA 2012-10-01 07:07:49 +0000 42) :order => %w(firstname id), 399223da app/models/user.rb (Jean-Philippe Lang 2012-10-30 08:40:12 +0000 43) :setting_order => 3 de0e0f09 app/models/user.rb (Toshi MARUYAMA 2012-10-01 07:07:49 +0000 44) },
98. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 行ごとの変更履歴から辿る2 git show de0e0f09 commit de0e0f09a33b20170468d9fdc67c4b5b7ac085d2 Author: Toshi MARUYAMA <marutosijp2@yahoo.co.jp> Date: Mon Oct 1 07:07:49 2012 +0000 pin user format order at setting panel (#10937) git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@10542 e93f8b46-1217-0410-a6f0-8f06a7374b81 diff --git a/app/controllers/settings_controller.rb b/app/controllers/settings_controller.rb index 3d2f9c1..e3387e4 100644 --- a/app/controllers/settings_controller.rb +++ b/app/controllers/settings_controller.rb @@ -39,7 +39,8 @@ class SettingsController < ApplicationController redirect_to :action => 'edit', :tab => params[:tab] else @options = {} - @options[:user_format] = User::USER_FORMATS.keys.collect {|f| [User.current.name(f), f.to_s] } + user_format = User::USER_FORMATS.collect{|key, value| [key, value[:setting_order]]}.sort{|a, b| a[1] <= + @options[:user_format] = user_format.collect{|v| v[0]}.collect{|f| [User.current.name(f), f.to_s]} @deliveries = ActionMailer::Base.perform_deliveries @guessed_host_and_path = request.host_with_port.dup
99. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 コミットログを活用するために は コミットメッセージに有用な情報を含 めておかないといけない 「ユーザーがグループに所属してい ない時にログインに失敗する問題を 修正」 ✓ 「チケットの作成時に空のフィール ドを許容しないように変更」 ✓ 5W1Hのうちの「Why」
100. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 いいかげんなコミットメッセージ はログの有用性を損なう 「バグ修正」 ✓ 「テスト」 ✓ 「腹減った」 ✓
101. 質問？
103. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 変更の手順 変更対象のコードを把握する 1. 変更する 変更の計画を立てる a. 自動テストを書く b. 実際に変更を行う c. 2.
104. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 その前に…… 変更対象のコードを把握する 1. 自動テストを自分で動かす 2. 変更する 3. 元から壊れていた物を「自分が壊し た」と誤解するとドツボにはまる
105. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 変更の計画を立てる 「これを実現する」というゴールを 明確にしよう 関係のない事は可能な限り「やらない」 ✓ 別の問題によってブロックされている場合 は、そちらから片付ける ✓ ✓ 例：期日をメールで通知できるよう にする ✓
106. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 具体的な計画に落とし込む1 期日のメール通知に何が必要か？ メールを送信する処理 ✓ 期日の到来を検知する仕組み ✓ 設定画面 ✓ ✓
107. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 具体的な計画に落とし込む2 どのモジュールに実装を追加する？ どんなクラス名・メソッド名にする？ ✓ ✓
108. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 具体的な計画に落とし込む3 どうやってテストする？ ✓
109. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 自動テストを書く（TDD） 色々な入力パターンを期待通りに 処理できるか？ ✓ 空の値を渡すとどうなる？ ✓ 数値が期待されるフィールドに文字 列を渡すとどうなる？ ✓ ある基準値以上だったらこれをす る、というような処理に「基準値と等 しい値」を与えるとどうなる？ ✓
110. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 実際に変更を行う （やる）
111. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 コードを書いてみて分かる事も 多い 個の設計ではテストを書きにく い…… ✓ 初期化の準備に手間がかかりすぎ る…… ✓ など
112. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 テストを書きにくい＝設計がま ずい モジュールの粒度が大きすぎ（責任 範囲が広すぎ）ないか？ ✓ 暗黙的に与えられるパラメータに依 存していないか？/副作用が多すぎ ないか？ ✓ その初期化処理は本当に必要なの か？ 関係ない物に無駄に依存して いないか？ ✓
113. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 テストしにくい部分の例 指定時刻になった時の動作を試す には、実際にPCの時刻設定を変え ないとテストできない ✓ メール送信処理の動作を試したい が、メールサーバーに繋がらないと テストできない ✓
114. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 外部要因への依存部分を小さ く分けよう 時刻 「PCの時計が示す時刻」から 「指定の文字列で表される時刻」へ ✓ ✓ メール送信 「実際のメールサーバーとのSMTPでの通 信」から 「メール送信用モジュールの呼び出し」へ ✓ ✓
115. 既存のコードを変更する時の心得 - 　　〜どこを見ればいいかの勘所〜 Powered by Rabbit 2.2.0 外部要因を差し替える 時刻 テストの時は任意の時刻を引数で渡す ✓ ✓ メール送信 テストの時は、メール送信用モジュールの 振る舞いを真似るモジュールを使う ✓ ✓
116. 質問？