# Code Review: bigdecimal-2

**URL:** <https://www.ruby-forum.com/t/code-review-bigdecimal-2/150123>\
**Category:** IronRuby\
**Created:** [October 4, 2008, 12:39am UTC](https://www.ruby-forum.com/t/code-review-bigdecimal-2/150123 "2008-10-04T00:39:30Z")\
**Posts on this page:** 5\
**Page:** 1

<div class="post-metadata">

**Author:** ![John\_LSSIRONRUBY](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/j/4491bb/32.png) [@John\_LSSIRONRUBY](https://www.ruby-forum.com/u/John_LSSIRONRUBY)\
**Post date:** [October 4, 2008, 12:39am UTC](https://www.ruby-forum.com/t/code-review-bigdecimal-2/150123/1 "2008-10-04T00:39:30Z")

</div>

tfpt review “/shelveset:bigdecimal-2;REDMOND\jflam”  
Comment :  
BigDecimal contribution from Peter Bacon D.

---

<div class="post-metadata">

**Author:** ![John\_LSSIRONRUBY](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/j/4491bb/32.png) [@John\_LSSIRONRUBY](https://www.ruby-forum.com/u/John_LSSIRONRUBY)\
**Post date:** [October 4, 2008, 1:24am UTC](https://www.ruby-forum.com/t/code-review-bigdecimal-2/150123/2 "2008-10-04T01:24:32Z")

</div>

Code looks good. There might have been less churn to have CodeContext  
on each method now so that we can use it to get a Config later – but  
there’s also something to be said for making it work first.

Style consistency issues:  
The line endings in bigdecimal.cs appear to be a mix of LF and CRLF.  
They should all be CRLF.  
We tend to prefer not to say “this.varname = x”; if the symbol name  
conflicts, it should be changed if possible.

---

<div class="post-metadata">

**Author:** ![John\_LSSIRONRUBY](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/j/4491bb/32.png) [@John\_LSSIRONRUBY](https://www.ruby-forum.com/u/John_LSSIRONRUBY)\
**Post date:** [October 4, 2008, 2:03am UTC](https://www.ruby-forum.com/t/code-review-bigdecimal-2/150123/3 "2008-10-04T02:03:08Z")

</div>

One more note:

```
    // TODO: This static field need to be added to a hash on 

```

RubyExecutionContext  
private static readonly BigDecimal.Config \_config = new  
BigDecimal.Config();  
internal static BigDecimal.Config GetConfig() { return \_config;  
}

There is already API on RubyContext for what you need:

```
    public bool TryGetLibraryData(object key, out object value)
    public object GetOrCreateLibraryData(object key, Func<object> 

```

valueFactory)  
public bool TryAddLibraryData(object key, object value, out  
object actualValue)

Tomas

---

<div class="post-metadata">

**Author:** ![John\_LSSIRONRUBY](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/j/4491bb/32.png) [@John\_LSSIRONRUBY](https://www.ruby-forum.com/u/John_LSSIRONRUBY)\
**Post date:** [October 5, 2008, 7:25am UTC](https://www.ruby-forum.com/t/code-review-bigdecimal-2/150123/4 "2008-10-05T07:25:04Z")

</div>

I’m going ahead and removing the static field. Love the  
GetOrCreateLibraryData API! 🙂

Thanks,  
-John

---

<div class="post-metadata">

**Author:** ![John\_LSSIRONRUBY](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/j/4491bb/32.png) [@John\_LSSIRONRUBY](https://www.ruby-forum.com/u/John_LSSIRONRUBY)\
**Post date:** [October 4, 2008, 1:33am UTC](https://www.ruby-forum.com/t/code-review-bigdecimal-2/150123/5 "2008-10-04T01:33:53Z")

</div>

I noticed the file name casing is wrong: bigdecimal.cs, fraction.cs. Was  
it broken by some transformation?

Tomas