Prevent callback from being set twice.

When you add one callack in two separate `set_callback` calls - it is
only called once.

When you do it in one `set_callback` call - it is called twice.

This violates the principle of least astonishment for me. Duplicating
callback is usually an error. There is a correct and obvious way to do
anything without this "feature".

If you want to do

    before_save :clear_balance, :calculate_tax, :clear_balance

or whatever, you should better do

    before_save :carefully_calculate_tax

    def carefully_calculate_tax
      clear_balance
      calculate_tax
      clear_balance
    end

And this even opens gates for some advanced refactorings, unlike the
first approach.

My assumptions are:

- Principle of least astonishment is violated, when callbacks are either
  prevented from duplication, or not.
- Duplicating callbacks is usually an error. When it is intentional -
  it's a smell of a bad design and can be approached without abusing
  this "feature".

My suggestion is: do not allow duplicating callbacks in one callback
call, like it is not allowed in separate callbacks call.

1 files changed, 9 insertions, 0 deletions

diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index 7b93731917..2fd5e0318f 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,5 +1,14 @@
 ## Rails 4.0.0 (unreleased) ##
 
+*   Prevent `Callbacks#set_callback` from setting the same callback twice.
+
+        before_save :foo, :bar, :foo
+
+    will at first call `bar`, then `foo`. `foo` will no more be called
+    twice.
+
+    *Dmitriy Kiriyenko*
+
 *   Deprecate `Time.time_with_date_fallback`, `Time.utc_time` and `Time.local_time`.
     These methods were added to handle the limited range of Ruby's native Time
     implementation. Those limitations no longer apply so we are deprecating them in 4.0