# Code Review: RubyClrInterop06

**URL:** <https://www.ruby-forum.com/t/code-review-rubyclrinterop06/154064>\
**Category:** IronRuby\
**Created:** [November 25, 2008, 12:23am UTC](https://www.ruby-forum.com/t/code-review-rubyclrinterop06/154064 "2008-11-25T00:23:39Z")\
**Posts on this page:** 4\
**Page:** 1

<div class="post-metadata">

**Author:** ![curthibbs](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/c/ac91a4/32.png) [@curthibbs](https://www.ruby-forum.com/u/curthibbs)\
**Post date:** [November 25, 2008, 12:23am UTC](https://www.ruby-forum.com/t/code-review-rubyclrinterop06/154064/1 "2008-11-25T00:23:39Z")

</div>

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

---

<div class="post-metadata">

**Author:** ![curthibbs](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/c/ac91a4/32.png) [@curthibbs](https://www.ruby-forum.com/u/curthibbs)\
**Post date:** [November 25, 2008, 1:04am UTC](https://www.ruby-forum.com/t/code-review-rubyclrinterop06/154064/2 "2008-11-25T01:04:02Z")

</div>

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

---

<div class="post-metadata">

**Author:** ![curthibbs](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/c/ac91a4/32.png) [@curthibbs](https://www.ruby-forum.com/u/curthibbs)\
**Post date:** [November 25, 2008, 2:47am UTC](https://www.ruby-forum.com/t/code-review-rubyclrinterop06/154064/3 "2008-11-25T02:47:31Z")

</div>

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;

---

<div class="post-metadata">

**Author:** ![curthibbs](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/c/ac91a4/32.png) [@curthibbs](https://www.ruby-forum.com/u/curthibbs)\
**Post date:** [November 25, 2008, 2:54am UTC](https://www.ruby-forum.com/t/code-review-rubyclrinterop06/154064/4 "2008-11-25T02:54:31Z")

</div>

Alright then 🙂