adamwiggins / rush

Ruby replacement for bash+ssh
http://rush.heroku.com/
518 stars 73 forks source link

Extending Array #2

Open trans opened 15 years ago

trans commented 15 years ago

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 commented 12 years ago

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 commented 12 years ago

+1 for every Array extension method it defines

scytacki commented 12 years ago

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 commented 12 years ago

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

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