Project

General

Profile

Bug #2495

Matrix: Vector#each2 should check its argument

Added by marcandre (Marc-Andre Lafortune) over 9 years ago. Updated about 8 years ago.

Status:
Closed
Priority:
Normal
Target version:
ruby -v:
ruby 1.9.2dev (2009-12-19 trunk 26121) [x86_64-darwin10.2.0]
Backport:
[ruby-core:27223]

Description

=begin
$ rubydev -r matrix -e 'p Vector[*1..4].each2(nil){|x, y| p "#{x}, #{y}"}'
/usr/local/rubydev/lib/ruby/1.9.1/matrix.rb:1149:in each2': undefined methodsize' for nil:NilClass (NoMethodError)

$ rubydev -r matrix -e 'p Vector[*1..4].each2(42){|x, y| p "#{x}, #{y}"}'
/usr/local/rubydev/lib/ruby/1.9.1/matrix.rb:1149:in each2': Vector dimension mismatch (ExceptionForMatrix::ErrDimensionMismatch)
from -e:1:in
'

$ rubydev -r matrix -e 'p Vector[*1..8].each2(42){|x, y| p "#{x}, #{y}"}'
"1, 0"
"2, 1"
"3, 0"
"4, 1"
"5, 0"
"6, 1"
"7, 0"
"8, 0"

(or vice versa, if on a 32 bit platform)
=end

Associated revisions

Revision cd492563
Added by keiju (Keiju Ishitsuka) about 9 years ago

  • lib/matrix.rb(Vector#each2, Vector#collect2): add type check for Integer[Bug #2495].

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@27091 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 27091
Added by keiju (Keiju Ishitsuka) about 9 years ago

  • lib/matrix.rb(Vector#each2, Vector#collect2): add type check for Integer[Bug #2495].

Revision 27091
Added by keiju (Keiju Ishitsuka) about 9 years ago

  • lib/matrix.rb(Vector#each2, Vector#collect2): add type check for Integer[Bug #2495].

Revision 27091
Added by keiju (Keiju Ishitsuka) about 9 years ago

  • lib/matrix.rb(Vector#each2, Vector#collect2): add type check for Integer[Bug #2495].

Revision 27091
Added by keiju (Keiju Ishitsuka) about 9 years ago

  • lib/matrix.rb(Vector#each2, Vector#collect2): add type check for Integer[Bug #2495].

Revision 27091
Added by keiju (Keiju Ishitsuka) about 9 years ago

  • lib/matrix.rb(Vector#each2, Vector#collect2): add type check for Integer[Bug #2495].

Revision 27091
Added by keiju (Keiju Ishitsuka) about 9 years ago

  • lib/matrix.rb(Vector#each2, Vector#collect2): add type check for Integer[Bug #2495].

History

#1

Updated by mame (Yusuke Endoh) over 9 years ago

=begin
Hi,

2009/12/19 Marc-Andre Lafortune redmine@ruby-lang.org:

Bug #2495: Matrix: Vector#each2 should check its argument
http://redmine.ruby-lang.org/issues/show/2495

Author: Marc-Andre Lafortune
Status: Open, Priority: Normal
Assigned to: Keiju Ishitsuka, Category: lib, Target version: 1.9.2
ruby -v: ruby 1.9.2dev (2009-12-19 trunk 26121) [x86_64-darwin10.2.0]

$ rubydev -r matrix -e 'p Vector[*1..4].each2(nil){|x, y| p "#{x}, #{y}"}'
/usr/local/rubydev/lib/ruby/1.9.1/matrix.rb:1149:in each2': undefined methodsize' for nil:NilClass (NoMethodError)

$ rubydev -r matrix -e 'p Vector[*1..4].each2(42){|x, y| p "#{x}, #{y}"}'
/usr/local/rubydev/lib/ruby/1.9.1/matrix.rb:1149:in each2': Vector dimension mismatch (ExceptionForMatrix::ErrDimensionMismatch)
       from -e:1:in
'

$ rubydev -r matrix -e 'p Vector[*1..8].each2(42){|x, y| p "#{x}, #{y}"}'
"1, 0"
"2, 1"
"3, 0"
"4, 1"
"5, 0"
"6, 1"
"7, 0"
"8, 0"

Agreed.

diff --git a/lib/matrix.rb b/lib/matrix.rb
index e6f5fe1..3a8c76a 100644
--- a/lib/matrix.rb
+++ b/lib/matrix.rb
@@ -1144,6 +1144,9 @@ class Vector
# Iterate over the elements of this vector and +v+ in conjunction.
#
def each2(v) # :yield: e1, e2

  • unless v.is_a?(Array) || v.is_a?(Vector)
  • Vector.Raise TypeError, v.class, "Array or Vector"
  • end Vector.Raise ErrDimensionMismatch if size != v.size return to_enum(:each2, v) unless block_given? size.times do |i|

--
Yusuke ENDOH mame@tsg.ne.jp

=end

#2

Updated by mame (Yusuke Endoh) over 9 years ago

  • Status changed from Open to Rejected

=begin
According to [ruby-dev:40237] and [ruby-dev:40267], it is the fate of
duck typing and compatibility.

[ruby-dev:40237] by Keiju (the author of lib/matrix.rb):

  • Integer#[] and #size have wrong names from the aspect of duck typing.
  • Type checking to restrict only Vector and Array will prevent NVector of NArray.

[ruby-dev:40267] by matz:

  • Integer#[] and #size are older than the concept of duck typing, and it is too late to change their names.
  • From the aspect of duck typing, type checking should not be done and we should accept such an error of [ruby-core:27901].

--
Yusuke ENDOH mame@tsg.ne.jp
=end

#3

Updated by marcandre (Marc-Andre Lafortune) over 9 years ago

  • Status changed from Rejected to Open

=begin
Hi

On Sun, Jan 31, 2010 at 10:12 AM, Yusuke Endoh redmine@ruby-lang.org wrote:

According to [ruby-dev:40237] and [ruby-dev:40267], it is the fate of
duck typing and compatibility.

Thank you for relaying this discussion.

I am all in favor of duck typing. I believe there is a solution here, which would be to insure that the argument responds to :each (or to :to_a.) Indeed, the name of the method is :each2, so I feel it is quite reasonable to request that the argument responds to :each. This would solve the issue for both Integer and String.

Finally, I feel it would be superior to raise a TypeError instead of a NoMethodError in a case where duck typing fails, in a similar fashion to "$stdout = nil" which raises TypeError: $stdout must have write method, NilClass given

So I'd recommend something like:

diff --git a/lib/matrix.rb b/lib/matrix.rb
index e6f5fe1..3a8c76a 100644
--- a/lib/matrix.rb
+++ b/lib/matrix.rb
@@ -1144,6 +1144,9 @@ class Vector
# Iterate over the elements of this vector and +v+ in conjunction.
#
def each2(v) # :yield: e1, e2

  • unless v.respond_to?(:each) && v.respond_to?(:size) && v.respond_to?(:[])
  • raise TypeError, "Argument should be array-like"
  • end Vector.Raise ErrDimensionMismatch if size != v.size return to_enum(:each2, v) unless block_given? size.times do |i|

=end

#4

Updated by mame (Yusuke Endoh) over 9 years ago

=begin
Hi,

2010/2/2 Marc-Andre Lafortune redmine@ruby-lang.org:

Finally, I feel it would be superior to raise a TypeError instead of a NoMethodError in a case where duck typing fails, in a similar fashion to "$stdout = nil" which raises TypeError: $stdout must have write method, NilClass given

I really understand your intuition and even wrote the patch once,
but to achieve it, I guess that very many type checks will be needed,
not only in Vector#each2 but also in other libraries. It may not be
realistic.
I think that `the fate of duck typing' includes such a error.

Now that I think about it, a type means "what methods its instance
responds to." Thus, it may be strange to distinguish TypeError and
NoMethodError.

That's just my personal opinion, though.
Matz or Keiju, will you give us your opinion?

--
Yusuke ENDOH mame@tsg.ne.jp

=end

#5

Updated by marcandre (Marc-Andre Lafortune) about 9 years ago

=begin

Hi,

On Mon, Feb 1, 2010 at 10:44 PM, Yusuke ENDOH mame@tsg.ne.jp wrote:

Matz or Keiju, will you give us your opinion?

Ist Keiju is even registered on the ruby-core mailing list?

I'd like to point out an alternate implementation can use #each instead of #[], and at least avoid the non-sensical results on integers. Example:

diff --git a/lib/matrix.rb b/lib/matrix.rb
index d452bf4..f9b2f58 100644
--- a/lib/matrix.rb
+++ b/lib/matrix.rb
@@ -1146,8 +1146,10 @@ class Vector
def each2(v) # :yield: e1, e2
Vector.Raise ErrDimensionMismatch if size != v.size
return to_enum(:each2, v) unless block_given?

  • size.times do |i|
  • yield @elements[i], v[i]
  • i = 0
  • v.each do |elem|
  • yield @elements[i], elem
  • i+=1 end self end

I'm all for duck typing, as long as we give an idea of what type of "quacking" we're expecting. I would still suggest adding at the beginning of the method:

unless v.respond_to?(:size) && v.respond_to?(:each)
raise TypeError, "Argument must respond to :size and :each"
end

=end

#6

Updated by matz (Yukihiro Matsumoto) about 9 years ago

=begin
Hi,

In message "Re: [ruby-core:28399] [Bug #2495] Matrix: Vector#each2 should check its argument"
on Tue, 2 Mar 2010 16:52:31 +0900, Marc-Andre Lafortune redmine@ruby-lang.org writes:

|On Mon, Feb 1, 2010 at 10:44 PM, Yusuke ENDOH mame@tsg.ne.jp wrote:
|> Matz or Keiju, will you give us your opinion?
|
|Ist Keiju is even registered on the ruby-core mailing list?

|I'd like to point out an alternate implementation can use #each instead of #[], and at least avoid the non-sensical results on integers. Example:

Accepted, but I prefer each_with_index though.

|I'm all for duck typing, as long as we give an idea of what type of "quacking" we're expecting. I would still suggest adding at the beginning of the method:
|
|unless v.respond_to?(:size) && v.respond_to?(:each)
| raise TypeError, "Argument must respond to :size and :each"
|end

If we change the policy and decide to add respond_to? check to
methods, we add infinite number of checks everywhere (exaggerated of
course). I am not in a mood to make such big change during 1.9.2
period.

                        matz.

=end

#7

Updated by matz (Yukihiro Matsumoto) about 9 years ago

=begin
Hi,

In message "Re: [ruby-core:28401] Re: [Bug #2495] Matrix: Vector#each2 should check its argument"
on Tue, 2 Mar 2010 17:29:46 +0900, Yukihiro Matsumoto matz@ruby-lang.org writes:

||I'd like to point out an alternate implementation can use #each instead of #[], and at least avoid the non-sensical results on integers. Example:
|
|Accepted, but I prefer each_with_index though.

I tried, but test failed, because Vector does not know how to each.
I am not sure if it's sufficient to make Vector enumerable.

                        matz.

=end

#8

Updated by keiju (Keiju Ishitsuka) about 9 years ago

  • Status changed from Open to Closed

=begin
I think that there is not the necessity to change.
However, I acknowledge that it is wrong a result for Integer argument.
And it will happen often mistake Integer pass.

I add type check for Integer only.

=end

Also available in: Atom PDF