Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Monkey-patched Array#sum method changes/breaks Ruby 2.4 method functionality #58

Closed
sjdemartini opened this issue Apr 18, 2017 · 4 comments

Comments

@sjdemartini
Copy link

sjdemartini commented Apr 18, 2017

In lib/statsample.rb, the core Ruby Array class is monkey-patched to change the behavior of the sum method:

def sum
inject(:+)
end

Active Support (included by Rails) already defines behavior for #sum, which has slightly different behavior and can accept a block for evaluation: https://github.com/rails/rails/blob/3d716b9e66e334c113c98fb3fc4bcf8a945b93a1/activesupport/lib/active_support/core_ext/enumerable.rb#L2-L27

Similarly, the Ruby core library added the #sum method to the Enumerable class in Ruby 2.4:

As such, for example, the following code works both with Active Support and/or Ruby 2.4:

> x = ['foo', 'bar']
#=> ["foo", "bar"]
> x.sum(&:bytesize)
#=> 6

But fails when the statsample 2.0 library is included in the project's Gemfile:

> x = ['foo', 'bar']
#=> ["foo", "bar"]
> x.sum(&:bytesize)
#=> "foobar"

Can this monkey-patching be removed and the suming functionality be done directly only where needed? This is preventing me from using statsample, unfortunately.

@NOX73
Copy link

NOX73 commented Jun 30, 2017

🤦‍♂️
+1

@v0dro
Copy link
Member

v0dro commented Jul 3, 2017

Can you send a PR with the monkey patching removed and replace instances of Array#sum with an appropriate method?

@sjdemartini
Copy link
Author

I don't have the time myself to fix this, unfortunately.

@IsmailM
Copy link

IsmailM commented Sep 12, 2017

We had this issue as well - We simply patched over it again after requiring statsample - not ideal (or clean) but solved the issue for us.

Imo It would be best to avoid global monkey patch - they will have unintended side-effects and break other ruby functions and make it hard to debug when they go wrong (I had no idea it was Statsample monkey-patching when I first saw this issue)...

Any chance this could be implemented as a Refinement -

Alternatively, at the very least, put them into a separate class/module and then use inheritance to monkey-patch.

One potential implementation would be:

module StatSample
  module ArrayExtensions
    def sum
      ....
    end 
  end 
end 

class Array
   include StatSample::ArrayExtensions
end

See https://www.justinweiss.com/articles/3-ways-to-monkey-patch-without-making-a-mess/

@v0dro v0dro closed this as completed in c605013 Oct 2, 2017
v0dro added a commit that referenced this issue Oct 2, 2017
Don't define Array#sum if already defined. Fixes #58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants