[Ironruby-core] Code Review: RubyClrInterop06

Curt Hagenlocher curth at microsoft.com
Mon Nov 24 20:46:24 EST 2008


Unfortunately, GetParameters() doesn't return a type array -- it returns a ParameterInfo array.  Here we see the evil of "var" -- it hides the actual type :).

I'll look for opportunities to split the method;

-----Original Message-----
From: Tomas Matousek
Sent: Monday, November 24, 2008 4:03 PM
To: Curt Hagenlocher; IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
Subject: RE: Code Review: RubyClrInterop06

In DefineConstructors, newParams seems to be unnecessarily copied.
Wouldn't it be better to do:

Type[] newParams;
Type[] baseParams = baseCtor.GetParameters();

if (has ruby class) {
  newParams = baseParams;
} if (is serializer) {
  newParams = baseParams;
} else {
  ...
  newPrams = ArrayUtils.Insert(typeof(RubyClass), baseParams);
}

?

I would also split this method to multiple pieces, seems too big already.

Other than that, looks good!

Tomas

-----Original Message-----
From: Curt Hagenlocher
Sent: Monday, November 24, 2008 3:23 PM
To: IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
Subject: Code Review: RubyClrInterop06

tfpt review "/shelveset:RubyClrInterop06;REDMOND\curth"
Comment  :
Create constructors on generated types that match each base class constructor Allocator logic not yet updated to use new constructors

--
Curt Hagenlocher
curth at microsoft.com


More information about the Ironruby-core mailing list