（2021-8-28追記）
この記事の改訂版を書いてみました。改訂版の方が易しい内容になっているので、プログラミング初心者の方はこちらを参考にしてみてください。
はじめに：「引数があるよりは、ない方が良い」？
先日、同僚の西見さん（@mah_lab）がこんな技術ブログを書いていました。
インスタンスメソッドとクラスメソッドはどのようにして使い分けるべきか？（Rubyの場合）
同じ内容を僕だったらどういうふうに書くかな～？と思って、ちょっと書き始めてみたんですが、わかりやすく実践的な説明をするのは意外と難しく、内容も西見さんのブログとほぼ同じになりそうだったので、途中で断念しました。
というわけで、インスタンスメソッドとクラスメソッドの使い分けが未だにあやふやだという方は、ぜひ西見さんのブログを読んでみてください！
・・・なんですが、1点だけ気になる点がありました。
それはインスタンスメソッドの利点に関する以下の記述です。
クラスの外側でのデータの受け渡しはなくなり、データの受け渡しはクラス内部で行われるようになりました。クラスの中で十分な情報を蓄えているため、renderメソッドは引数を取る必要がありません。
(中略)
引数があるよりは、ない方が良いのです。引数がなければ、そもそも何を情報として渡さなければいけないかを考えなくて済みます。引数があることで、コードを使う人に余計な仕事をさせることになります。
ブログの中で使われているサンプルコードを見れば、その利点は明らかで疑問の余地はありません。
ただし、「引数があるよりは、ない方が良いのです。」という判断基準を機械的に適用してしまうと、かえって扱いづらいプログラムが出来上がる可能性があります。
というわけで、この投稿では「引数があるよりは、ない方が良いのです。」を誤解した人のサンプルコードを挙げて、何がダメなのかを説明します。
ダメなコード例
ここでは架空の販売管理システムを考えてみます。
このシステムは1件の注文に対する注文明細をCSVから読み込んで作成できる、という機能があります。
初心者プログラマのAさんは「引数があるよりは、ない方が良い」というルールを機械的に適用して次のようなコードを書きました。
class Order
attr_reader :result
def initialize
@csv_data = nil
@result = []
end
def read_csv(file_path)
@csv_data = CSV.read(file_path)
end
def save_details
@csv_data.each do |row|
detail = OrderDetail.new
# 注文明細を作成する処理...
if detail.valid?
@result << true
else
@result << '作成に失敗しました。'
end
end
end
end
読み込んだCSVファイルのデータはインスタンス変数に格納したので、save_detailsメソッドには引数がありません。やった！
・・・と思わないようにしましょう。
このコードを利用するときはどんなコードになるか？
実際に上のコードを利用する場合を想定してみましょう。
おそらく次のようになると思います。
# 注文データを作成する
order = create_order
# 明細データのCSVを読み込む
order.read_csv(file_path)
# 読み込んだデータを元に注文明細を作成する
order.save_details
# 作成の結果を受け取る
result = order.result
問題点1: メソッドを呼び出す順番を間違えるとエラーになる
save_detailsメソッドは、事前にCSVデータが読み込まれていることを前提としています。
なので、次のような呼び出し方をするとエラーになります。
order = create_order
order.save_details # @csv_dataがnilなのでエラーが起きる
同様に、resultを読み出す場合も先に処理を実行していないと中身が空っぽです。
order = create_order
result = order.result # 中身が空
このクラスを作ったAさんは「先に必要なメソッドを呼び出さない方が悪い！」と考えるかもしれませんが、こういうケースは呼び出す順番（＝コンテキスト）に依存するメソッドを作ってしまったAさんが悪いです。
クラス設計は「使う側に気を遣わせる」のではなく、 「作る側が気を遣う」のが原則 です。
問題点2: save_detailsを2回呼び出すとresultが追記されてしまう
実際にこんな呼び出し方をする人はいないと思いますが、もし save_details を2回呼び出すと、@result の内容がどんどん追記されてしまい、使い物にならなくなります。
order = create_order
order.read_csv(file_path)
order.save_details
order.save_details
result = order.result
「あー、ごめんなさい。save_detailsメソッドを修正しておいたんでこれで大丈夫です！」と、Aさんはコードを修正してきました。
def save_details
@result = [] # resultを毎回初期化！！
@csv_data.each do |row|
detail = OrderDetail.new
# 注文明細を作成する処理...
if detail.valid?
@result << true
else
@result << '作成に失敗しました。'
end
end
end
・・・はい、確かにここではこの対応でなんとかなりますね。
ですが、そもそもこのクラスの問題はもっと本質的なところにあります。
その根本的な問題を解決する方法を見ていきましょう。
改善策1: 一時的なデータは引数、戻り値、ローカル変数として受け渡す
このクラスの一番の問題は、処理の過程でしか必要とされない一時的なデータをインスタンス変数（@csv_data, @result）に格納してしまっていることです。
一時的なデータをインスタンス変数に入れるとクラスの利用者が不便になるだけでなく、そのクラスに手を加えるときもインスタンス変数が「クラス内のグローバル変数」になってしまい、クラスが大きくなるにつれて保守性がどんどん低下します。
こういった一時的なデータはメソッドの引数と戻り値、またはローカル変数として受け渡す必要があります。
改善策2: 1つの要求につき1メソッドにする
もうひとつの問題はクラスの利用者から見たときの1つの要求が、複数のメソッドに分かれていることです。
前述の例の場合、クラスの利用者が要求は「1件の注文に対する注文明細をCSVから読み込んで作成すること」でした。
しかし、この要求を実現するためには3つのメソッドが必要になっています。（order.read_csv、order.save_details、order.result）
しかも決まった順番で呼び出さないと適切な結果が得られません。
クラスの利用者の利便性を考えれば、こうした処理は1つのメソッドで完結させるべきです。
改善後のコード
ここまでに見てきた問題点と改善策を踏まえてコードを書き直すと、以下のようになります。
class Order
def create_details_from_csv(file_path)
result = []
csv_data = CSV.read(file_path)
csv_data.each do |row|
# (明細データを作成する処理)
if ok
result << true
else
result << '作成に失敗しました。'
end
end
result
end
end
これにより、呼び出す側のコードは次のように変わります。
order = create_order
result = order.create_details_from_csv(file_path)
1つのメソッドで「注文明細をCSVから読み込んで作成すること」が実現できているので、メソッドを呼び出す順番に気を遣う必要はありません。
処理の結果は戻り値として返ってくるようになりました。
最初のダメなコード（下記）に比べると「クラスの利用者にとって優しい設計」になっているのがわかると思います。
order = create_order
order.read_csv(file_path)
order.save_details
result = order.result
余談：「create_details_from_csv もまだ改善できそうですけど？」
改善後の create_details_from_csv も、これはこれでまだ改善できそうな点があると思います。
ですが、今回の記事は「インスタンス変数の間違った使い方を理解してもらう」というテーマにしているので、これ以上深入りしません。
どこをどう改善できそうかは隣の席の同僚と議論してみてください。
2014.11.17 7:30追記: 上級プログラマのみなさんへ
ここで紹介したサンプルコードはあくまで「インスタンス変数を正しく使わないとどうなるか」を示すために作成した架空のサンプルコードです。
また、初心者の方が理解しやすくなるように、説明上必要のない項目はすべて省略しています。
なので、サンプルコードの中身を具体的にレビューすると「ここが変」「ここはこうすべき」という点がいくつも出てくると思います（典型的な例でいうと「Orderは状態を持ってないから create_details_from_csv はクラスメソッドにすべきだ！」という指摘です）。
具体的なコードを見せられるとついつい具体的な指摘をしたくなる気持ちは十分理解できるのですが、「これはあくまで説明用のサンプルコードだから」という温かい目で見てもらえるとありがたいです。
が！
架空のコードではなく、実際に動くコードでこれをやったらどうなるのか確認してもらうために、このサンプルアプリケーションをRailsで実際に作ってみました。
インスタンス変数を使う場合と使わない場合のdiffはこちらで確認できます。
このRailsのコードを見れば、create_details_from_csv メソッドがインスタンスメソッドのままでも問題ないことを理解してもらえるんじゃないかと思います。
まとめ
というわけで、今回はクラスの利用者にも保守担当者にも迷惑をかける「インスタンス変数の間違った使い方」とその改善策を紹介してみました。
ちなみに「ダメなコード例」で見せたようなコードは、前職でも前々職でもよく見かけたコードです。
もしかすると、あなたの周りでも同じようなコードを書いている人がいるかもしれません。（それがあなたではないことを祈りますが）
その場合は職場内でコードレビューをしたり、オブジェクト指向設計に関する技術書の読書会を開いたりして、「みんなが幸せになるクラス」をチーム全員が作れるようになりましょう！
あわせて読みたい
プログラマ歴12年の僕が選んだ「10年経っても役立つ技術書17選」
きれいなコードを書いたり、きちんとしたクラスを設計したりするのに役立つ（僕は役に立った）技術書を紹介しています。
[初心者向け] RubyやRailsでリファクタリングに使えそうなイディオムとか便利メソッドとか
Rubyらしい効率のいい書き方を学びたい方はこちらの記事が役に立つかもしれません。
レガシープログラマかどうかを判断する10項目
「7. クラスのフィールド変数をグローバル変数のように利用する。」は今回紹介した内容とほぼ同じです。