Looks great!

Could you copy the comment "# MRI follows hashing semantics here, so doesn't 
actually call eql?/hash for Fixnum/Symbol" from core\array\minus_spec.rb to 
IListOps.Difference as its not obvious why the code in IListOps.Difference does 
the complicated checks.

Yes, it seems OK that you can call all the Array methods on 
System::Collections::ArrayList (and any System::Collectinos::IList object in 
general). We are exposing Ruby String methods on System::String, and it would 
be consistent with that approach.

About List<T>.Reverse, you could argue that if you use Ruby casing of 
"reverse", IListOps should get precedence. Could you open a bug for this 
please? If you use "Reverse", then the CLR method should get precedence, and 
there would be a possibility of confusion, but as you say, there is not much 
that can be done. It does not seem worth special-casing.

Thanks,
Shri


-----Original Message-----
From: ironruby-core-boun...@rubyforge.org 
[mailto:ironruby-core-boun...@rubyforge.org] On Behalf Of Daniele Alessandri
Sent: Monday, April 20, 2009 12:46 PM
To: ironruby-core@rubyforge.org
Subject: [Ironruby-core] More fixes for core/array specs (again)

Hi,
I have attached a diff with new fixes for a whole bunch of failing expectations 
in the core/array specs.

* Slightly modified IListOps.UniqueSelf as suggested upon review.
* Fixed IListOps.ReverseIndex as Array#rindex should not fail if elements are 
removed from the array during the iteration over its elements.
* Changed IListOps.Reverse to return IList instances of the same type of the 
given self argument (this change also fixes the following failing spec: 
"Array#reverse returns subclass instance on Array
subclasses")
* Changed IListOps.SetElement to make it try to invoke #to_ary on its argument 
for multi-element sets.
* Changed one overload of IListOps.Equals to make it try to call #to_ary on the 
object argument of Array#== and use the resulting array as the actual argument 
for the equality check.
* Cleaning up tags removing expectations that do not fail anymore.
* Added ArrayOps.ToAry as Array#to_a and Array#to_ary behave differently on 
subclasses of arrays.
* Various changes to IListOps.Join to clear all of the remaining tags for the 
specs of Array#join. The tags marked as critical in join_tags.txt are not 
related to pending bugs for Array#join.
* Changed IListOps.Repetition to return IList instances of the same type of the 
given self argument (this fixes also "Array#* with an integer returns subclass 
instance with Array subclasses")
* Modified IListOps.RecursiveJoin to make it flag the resulting string as 
tainted if the given array, at least one of its elements or the separator 
string are tainted.
* IListOps.Difference now uses Object#hash and Object#eql? to check for object 
equality, this fixes the failing spec "Array#- acts as if using an intermediate 
hash to collect values"

These changes directly clear the following failing expectations:

Array#[]= calls to_ary on its rhs argument for multi-element sets Array#== 
calls to_ary on its argument Array#join tries to convert the passed seperator 
to a String using #to_str Array#join checks whether the passed seperator 
responds to #to_str Array#join handles recursive arrays Array#join does not 
consider taint of either the array or the separator when the array is empty 
Array#join returns a string which would be infected with taint of the array, 
its elements or the separator when the array is not empty Array#join does not 
process the separator if the array is empty Array#join raises a TypeError if 
the passed separator is not a string and does not respond to #to_str
Array#- acts as if using an  intermediate hash to collect values
Array#* with an integer returns subclass instance with Array subclasses
Array#* with an integer copies the taint status of the original array even if 
the array is empty
Array#* with an integer copies the taint status of the original array if the 
passed count is not 0
Array#* with a string returns a string which would be infected with taint of 
the array, its elements or the separator when the array is not empty 
Array#reverse returns subclass instance on Array subclasses Array#rindex does 
not fail when removing elements from block

I have only a question: I've made changes to IListOps.Reverse and 
IListOps.Repetition in order to make them return IList instead of RubyArray. In 
ruby, Array#reverse and Array#* maintain the same type of self for the 
returning object (that is, if self is a RubyArray.Subclass then it is returned 
a new object of the same type).
With these changes we would actually be able to do something like
this:

>>> arraylist = System::Collections::ArrayList.new([1,2,3,4,5])
=> [1, 2, 3, 4, 5]
>>> multiplied_arraylist = arraylist * 2
=> [1, 2, 3, 4, 5, 1, 2, 3, 4, 5]
>>> multiplied_arraylist.class
=> System::Collections::ArrayList

Would it be a consistent behaviour?

On a (partially) unrelated note, I've just noticed that List<T>.Reverse is more 
like Array#reverse! but it actually take the precedence over IListOps.Reverse. 
I'm just thinking if this could lead to some sort of confusion (ohan I'm not 
sure if anything can be done about this.

Thanks,
Daniele


PS: I'm waiting to push these changes on my remote repository, Jim told he is 
going to pull today but I would like to wait for a review first (and yeah, 
today I'm too lazy to actually start off a new branch :-)).

--
Daniele Alessandri
http://www.clorophilla.net/blog/
http://twitter.com/JoL1hAHN
_______________________________________________
Ironruby-core mailing list
Ironruby-core@rubyforge.org
http://rubyforge.org/mailman/listinfo/ironruby-core

Reply via email to