Extending Array #2

Open
trans opened this Issue Sep 19, 2009 · 4 comments

Projects

None yet

3 participants

@trans
trans commented Sep 19, 2009

Hi, I'm looking at using Rush for on my projects, and if I do I would like to contribute back to Rush, as I will need many more commands than Rush currently defines.

I need to look into it a bit more, but I do see one issue I would want to see addressed, that is the extension of the Array class. There is the potential here for conflicts with the built-in Array methods and other people's Array extensions (of which I use quite a few). A more robust approach would be to use a subclass of Array, for the sake of discussion, let's call it FileArray. The Rush methods would return a Rush::FileArray instead of an Array where applicable. A single method can be added to Array to convert a regular array to a FileArray, perhaps #files or #to_files.

@numbers1311407

Seems possibly futile since this is a 2 year old issue, but I strongly second this. Rush is really cool, but the way it clutters up Array is just messy and error prone.

I ran into a real-life case of this just now on a Rails 3 project on Heroku. The method_missing implementation of ActiveRecord::Relation looks to methods defined on Array first before scopes, meaning that, for example, Rush:Commands#search would short circuit an ActiveRecord scope of the same name.

The result was the evocation of a few "What the hell??" exclamations when I started seeing errors complaining about my class not responding to dir? in the Heroku logs. It was only then that I realized Heroku was including Rush and mixing all kinds of methods into Array.

A Rush::FileArray or something would be a great thing, as opposed to the assumption that all Arrays in the app are lists of files.

@trans
trans commented Mar 14, 2012

+1 for every Array extension method it defines

@scytacki

I just found that including the rush gem in my Rails 3.2 app slows down large queries by around 3.5 times. I haven't narrowed it down, but I'm guessing it is these Array extensions that cause the slow down. For some real data, here is a benchmark that collects around 13k AR objects:
without rush gem:
1.9.3p125 :001 > Benchmark.measure { District.all }
=> 0.990000 0.060000 1.050000 ( 1.081643)

with rush 0.6.8 gem:
1.9.3p125 :001 > Benchmark.measure { District.all }
=> 3.280000 0.080000 3.360000 ( 3.398285)

@trans
trans commented Jul 28, 2012

Let me be a little more adamant here: +10E100!

If you want Rush to a successful app/library make a FileArray and use it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment