I'm going ahead and removing the static field. Love the GetOrCreateLibraryData API! :)
Thanks, -John -----Original Message----- From: Tomas Matousek Sent: Friday, October 03, 2008 4:38 PM To: Curt Hagenlocher; John Lam (IRONRUBY); IronRuby External Code Reviewers Cc: [email protected] Subject: RE: Code Review: bigdecimal-2 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 -----Original Message----- From: Curt Hagenlocher Sent: Friday, October 03, 2008 4:03 PM To: John Lam (IRONRUBY); IronRuby External Code Reviewers Cc: [email protected] Subject: RE: Code Review: bigdecimal-2 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. -----Original Message----- From: John Lam (IRONRUBY) Sent: Friday, October 03, 2008 3:24 PM To: IronRuby External Code Reviewers Cc: [email protected] Subject: Code Review: bigdecimal-2 tfpt review "/shelveset:bigdecimal-2;REDMOND\jflam" Comment : BigDecimal contribution from Peter Bacon Darwin _______________________________________________ Ironruby-core mailing list [email protected] http://rubyforge.org/mailman/listinfo/ironruby-core
