Forum: IronRuby Code Review: bigdecimal-2

Announcement (2017-05-07): www.ruby-forum.com is now read-only since I unfortunately do not have the time to support and maintain the forum any more. Please see rubyonrails.org/community and ruby-lang.org/en/community for other Rails- und Ruby-related community platforms.
A120eb09d383829bfa210bf656fcde4e?d=identicon&s=25 John Lam (IRONRUBY) (Guest)
on 2008-10-04 00:39
(Received via mailing list)
Attachment: bigdecimal-2.diff (200 KB)
tfpt review "/shelveset:bigdecimal-2;REDMOND\jflam"
Comment  :
  BigDecimal contribution from Peter Bacon Darwin
F983f0c990cba2fe743ef62a975ec99c?d=identicon&s=25 Curt Hagenlocher (Guest)
on 2008-10-04 01:24
(Received via mailing list)
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.
Cb51033949ffccd982ae32c9f890f25a?d=identicon&s=25 Tomas Matousek (Guest)
on 2008-10-04 01:33
(Received via mailing list)
I noticed the file name casing is wrong: bigdecimal.cs, fraction.cs. Was
it broken by some transformation?

Tomas
Cb51033949ffccd982ae32c9f890f25a?d=identicon&s=25 Tomas Matousek (Guest)
on 2008-10-04 02:03
(Received via mailing list)
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
A120eb09d383829bfa210bf656fcde4e?d=identicon&s=25 John Lam (IRONRUBY) (Guest)
on 2008-10-05 07:25
(Received via mailing list)
I'm going ahead and removing the static field. Love the
GetOrCreateLibraryData API! :)

Thanks,
-John
This topic is locked and can not be replied to.