Re: digest.so implemented

Wayne K.:

Attached is an implementation of the methods and classes from external

library digest.so that are used by some simple Rails use cases.

Thanks for the contribution! Sorry for such a long delay. It required
some additional work on our side and we also had some issues with TFS to
SVN sync tool. John fixed that last week, so your contribution have
finally been checked-in.

I’ve reviewed the code. While this feedback is specific to your
contribution, this is general advice that other contributors should
follow as well.

Check out the latest code in SVN - all my suggestions are reflected
there.

  1. Coding conventions:

    a) Use .NET Framework conventions for all identifiers (see
    Naming Guidelines - Framework Design Guidelines | Microsoft Learn). There is no
    specific guideline for naming private fields in this document; we prefix
    field names with underscores (e.g. private string _fooBar). If you’re
    not sure about some convention try to find out in the rest of the
    IronRuby code or ask in the list.

    b) Use /!/ for method parameters and instance fields that should
    never be null. See http://research.microsoft.com/specsharp for details
    on Spec# annotations.

  2. Do not use public fields (Base::algorithm, buffer). Use properties if
    it is necessary to expose the field or private/internal visibility
    otherwise.

Also use readonly if the field is not mutated after the object is
constructed.

  1. You don’t need to declare Call_Foo methods in order to invoke a
    dynamic site unless the invocation is somehow more complex (involves
    some conversions etc.)

  2. Please test for corner cases, eg whether the methods accept null
    arguments etc. Mark the parameters appropriately ([NotNull] attribute,
    /!/ etc.), add precondition checks. Here’s an example:

[RubyMethod(“*”)]

public static RubyArray/!/ Repetition(IList/!/ self, int repeat) {

 if (repeat < 0) {

    throw RubyExceptions.CreateArgumentError("negative argument");

 }

 ...

}

Thanks,

Tomas

This was just dying out for a wiki page:
http://ironruby.rubyforge.org/wiki/wiki.pl?ContributionGuidelines

Pete

From: [email protected]
[mailto:[email protected]] On Behalf Of Tomas M.
Sent: Monday,24 March 24, 2008 20:14
To: [email protected]
Subject: Re: [Ironruby-core] digest.so implemented

Wayne K.:

Attached is an implementation of the methods and classes from external

library digest.so that are used by some simple Rails use cases.

Thanks for the contribution! Sorry for such a long delay. It required
some
additional work on our side and we also had some issues with TFS to SVN
sync
tool. John fixed that last week, so your contribution have finally been
checked-in.

I’ve reviewed the code. While this feedback is specific to your
contribution, this is general advice that other contributors should
follow
as well.

Check out the latest code in SVN - all my suggestions are reflected
there.

  1. Coding conventions:

    a) Use .NET Framework conventions for all identifiers (see
    Naming Guidelines - Framework Design Guidelines | Microsoft Learn). There is no
    specific guideline for naming private fields in this document; we prefix
    field names with underscores (e.g. private string _fooBar). If you’re
    not
    sure about some convention try to find out in the rest of the IronRuby
    code
    or ask in the list.

    b) Use /!/ for method parameters and instance fields that should
    never
    be null. See http://research.microsoft.com/specsharp for details on
    Spec#
    annotations.

  2. Do not use public fields (Base::algorithm, buffer). Use properties if
    it
    is necessary to expose the field or private/internal visibility
    otherwise.

Also use readonly if the field is not mutated after the object is
constructed.

  1. You don’t need to declare Call_Foo methods in order to invoke a
    dynamic
    site unless the invocation is somehow more complex (involves some
    conversions etc.)

  2. Please test for corner cases, eg whether the methods accept null
    arguments etc. Mark the parameters appropriately ([NotNull] attribute,
    /!/
    etc.), add precondition checks. Here’s an example:

[RubyMethod(“*”)]

public static RubyArray/!/ Repetition(IList/!/ self, int repeat) {

 if (repeat < 0) {

    throw RubyExceptions.CreateArgumentError("negative argument");

 }

 ...

}

Thanks,

Tomas

Or crying out, even!

From: [email protected]
[mailto:[email protected]] On Behalf Of Tomas M.
Sent: Monday,24 March 24, 2008 20:14
To: [email protected]
Subject: Re: [Ironruby-core] digest.so implemented

Wayne K.:

Attached is an implementation of the methods and classes from external

library digest.so that are used by some simple Rails use cases.

Thanks for the contribution! Sorry for such a long delay. It required
some
additional work on our side and we also had some issues with TFS to SVN
sync
tool. John fixed that last week, so your contribution have finally been
checked-in.

I’ve reviewed the code. While this feedback is specific to your
contribution, this is general advice that other contributors should
follow
as well.

Check out the latest code in SVN - all my suggestions are reflected
there.

  1. Coding conventions:

    a) Use .NET Framework conventions for all identifiers (see
    Naming Guidelines - Framework Design Guidelines | Microsoft Learn). There is no
    specific guideline for naming private fields in this document; we prefix
    field names with underscores (e.g. private string _fooBar). If you’re
    not
    sure about some convention try to find out in the rest of the IronRuby
    code
    or ask in the list.

    b) Use /!/ for method parameters and instance fields that should
    never
    be null. See http://research.microsoft.com/specsharp for details on
    Spec#
    annotations.

  2. Do not use public fields (Base::algorithm, buffer). Use properties if
    it
    is necessary to expose the field or private/internal visibility
    otherwise.

Also use readonly if the field is not mutated after the object is
constructed.

  1. You don’t need to declare Call_Foo methods in order to invoke a
    dynamic
    site unless the invocation is somehow more complex (involves some
    conversions etc.)

  2. Please test for corner cases, eg whether the methods accept null
    arguments etc. Mark the parameters appropriately ([NotNull] attribute,
    /!/
    etc.), add precondition checks. Here’s an example:

[RubyMethod(“*”)]

public static RubyArray/!/ Repetition(IList/!/ self, int repeat) {

 if (repeat < 0) {

    throw RubyExceptions.CreateArgumentError("negative argument");

 }

 ...

}

Thanks,

Tomas

Thanks!
-John