scientistproject / Scientist.net

A .NET library for carefully refactoring critical paths. It's a port of GitHub's Ruby Scientist library
MIT License
1.47k stars 95 forks source link

Support custom comparators #15

Closed akamud closed 8 years ago

akamud commented 8 years ago

Similar to Ruby's original code.

I'm willing to implement this, but I would like to discuss the preferred implementation. I see a few ways to do this in C#:

1. Use an optional parameter

We could make Science have an optional parameter Func<T, T, bool> that would be called at the end of the experiment if it is not null.
I implemented this in my fork, you can see the changes needed here.

2. Add a property in Scientist class

Very similar to # 1, but instead of passing an optional parameter you would make Scientist generic (Scientist<T>), having a property Func<T, T, bool> Comparer that could be set like an ObservationPublisher.
This would change the way the lib is used right now, as a type parameter would have to be passed in Scientist class but no parameter would be needed in the Science method anymore.

Old way:

var result = Scientist.Science<int>("success", experiment =>
...

New way

var result = Scientist<int>.Science("success", experiment =>
...

From my point of view this would make the code a little less clean, as the type would have to be used anytime the Scientist object is referenced.

3. Change Experiment<T> interface

Ruby's implementation of Comparator is inside the Experiment class, doing that in C# would make implementing the Compare method required, even for straightforward value types' comparison.
Maybe we could provide an abstract Experiment class that would be easier to implement (default Compare virtual method using default C# comparators?) allowing the user to override when needed?


What do you think? Does any of these make sense? Is there a simpler more idiomatic way of doing this in C#?

giulianob commented 8 years ago

I would opt for option #3. You can just have the default implementation in ExperimentBuilder perform a simple Equal but allow the user to set a different comparison function. It seems that would match the Ruby API the best.

darcythomas commented 8 years ago

So I made a thing https://github.com/Haacked/Scientist.net/pull/26

When you setup a test you optionally can pass in a Func<T, T, bool> or a IEqualityComparer<T> if neither are passed in the the default rules are applied

Specifically it checks like this:


        /// <summary>
        /// Checks if two ExperimentResults are equal
        /// </summary>
        /// <param name="controlResult">Control ExperimentResult</param>
        /// <param name="candidateResult">Candidate ExperimentResult</param>
        /// <returns>
        ///  Returns true if: 
        /// 
        /// 
        ///  The values of the observations are equal according to an IEqualityComparer&lt;T&gt; expression, if given 
        ///  The values of the observations are equal according to a comparison function, if given  
        ///  Both observations raised an exception with the same Type and message.
        ///  The values of the observation are both null 
        ///  The values of the observations are equal according to Ts IEquatable&lt;T&gt; implementation, if implemented 
        ///  The values of the observations are equal (using .Equals()) 
        ///  
        ///  Returns false otherwise. 
        /// </returns>

Thoughts? Comments?

haacked commented 8 years ago

Fixed by https://github.com/Haacked/Scientist.net/pull/29