お題

引数として文字列の配列が与えられたとき，それらをハイフンで繋いだ文字列を返すメソッドを書いてください。
配列の要素はすべて文字列であると仮定して構いません。
ただし，要素は空文字列の可能性があり，これを除いたものだけをハイフンで繋ぎます。

つまり，以下のような動作をするものとします：

p join_with_hyphen(["foo", "bar", "", "baz"])
# => "foo-bar-baz"

（メソッド名が英語として妥当でない気がしますが，英語が苦手なのでよく分かりません。得意な方は教えてください）

コード

def join_with_hyphen(strings)
  strings.delete("")
  strings.join("-")
end

問題点

このメソッドはお題のとおりに動作します。
しかし，引数のオブジェクトを破壊的に変更しているのは非常に悪い流儀です。残念ながらこういうコードをよく目にします。

join_with_hyphen を呼び出す側では，与えた実引数を他の目的にも使うかもしれません。
その場合，オブジェクトを勝手に変化させられると困るのです。
これは分かりにくいバグの原因になります。

def join_with_hyphen(strings)
  strings.delete("")
  strings.join("-")
end

ary = ["foo", "bar", "", "baz"]

p join_with_hyphen(ary)
# => "foo-bar-baz"

p ary.length
# => 3 （えっ?!）

改善

原則として，メソッドは与えられた引数に対して破壊的な操作を行うべきではない，と（初心者のうちは）覚えておきましょう。

今回のお題については Enumerable#grep_v を使って

def join_with_hyphen(strings)
  strings.grep_v("").join("-")
end

とすればいいでしょう。

一般に，

def foo(arg)
  # なんとかかんとか
end

というメソッドで，arg が指しているオブジェクトを加工して結果を得たいというとき，非破壊的メソッドを使えばいいのですが，破壊的メソッドを使う必要がある場合は

def foo(arg)
  arg = arg.dup # 複製を作る
  arg.some_destructive_method # 破壊的操作
  # なんとかかんとか
end

と書くことができます。

さて，与えられた文字列に対してさまざまな変換を順次施す，ということがよくあります。このようなとき，すべて非破壊的メソッドを使って

def foo(str)
  str.downcase
    .tr(chars1, chars2)
    .gsub(r1, s1)
    .gsub(r2, s2)
    .gsub(r3, s3)
    .gsub(r4, s4)
end

のように書くこともできますが，メソッドチェーンの中で次々と String オブジェクトが出来て，それら中間のオブジェクトがすべてガーベジ¹になってしまいます。
たいがいそれで問題無いのですが，foo が多数回呼ばれ，str が非常に巨大な文字列であるような場合は望ましくありません。

破壊的メソッドを用いれば無駄なオブジェクトの増加が避けられますが，String#dup を用いずとも

def foo(str)
  str = str.downcase
  str.tr!(chars1, chars2)
  str.gsub!(r1, s1)
  str.gsub!(r2, s2)
  str.gsub!(r3, s3)
  str.gsub!(r4, s4)
  str
end

のように最初だけ非破壊的メソッドを使えばいいと思います。

余談

この記事の「コード」節で掲げたコードは，うっかり引数を破壊した例でした。
しかし，引数の破壊的変更を意図したメソッドを設計することはあります。

組込みライブラリーでいうと，例えば ObjectSpace.#count_objects が該当します。
これは存在するオブジェクトを種類ごとに数えて，その結果をハッシュとして返すメソッドなのですが，このメソッドの実行によってハッシュが増えてしまうことを防ぐために，結果を格納するハッシュを引数で与えることができるようになっています。

永久に参照されることのないオブジェクト。 ↩

【Ruby のまずいコード】引数の扱い

お題

コード

問題点

改善

余談