scientistproject / Scientist.net

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

Support different return types #44

Open davezych opened 8 years ago

davezych commented 8 years ago

I'm not sure how scientist handle this (nor how ruby handles this situation), and I know you want to stay close to the ruby lib, but it could be useful to support different return types between the control and candidate. When refactoring there is no guarantee that the candidate methods will return the same type as the existing method, which could lead to hardships trying to experiment on them. Supporting 2 different types allows us to get around that issue, and we can check the differences between 2 different complex objects in the comparison.

A potential implementation:

internal class Experiment<TControl, TCandidate>
{
    Func<Task<TControl>> _control;
    Func<Task<TCandidate>> _candidate;

    public void Use(Func<Task<TControl>> control);
    public void Try(Func<Task<TCandidate>> candidate);

    public void Compare(Func<TControl, TCandidate, bool> comparison);
}
M-Zuber commented 8 years ago

This looks nice but I would say you should force a comparer to be provided. Maybe a third generic param?

Another point: to keep the class from getting out of hand provide the above as a sub class / sibling class of the existing Experiment class

haacked commented 8 years ago

Perhaps

public class Experiment<T> : Experiment<T, T>
{
}

but I would say you should force a comparer to be provided.

That's a good point. A comparer isn't necessary for Experiment<T> but should be for Experiment<T, C>.

haacked commented 8 years ago

Maybe a third generic param?

No, we could derive it from the existing. It'd be Func<TControl, TCandidate, TBool>. I think we could make it a runtime verification.

akamud commented 8 years ago

I had a prototype of this implemented in #25, it provided a way of hiding this complexity from experiments with same type.

I had this same doubt about how to enforce the user to provide a comparer function for these cases. I'm a big fan of not letting the user create objects with unstable states, even if we check it at runtime, it is not clear to the user that he needs to provide a comparer function. Maybe we could create a ComplexExperiment<TControl,TCandidate> class that would need a parameter with custom comparer. I know this is not perfect either, but it is an alternative.

M-Zuber commented 5 years ago

I am attempting to pick this back up (/me gains necromancer badge) but have run into a .NET snag In the intervening time, an interface with this signature was added:

/// <summary>
/// Provides an interface for defining a synchronous experiment that provides a clean value to publish.
/// </summary>
/// <typeparam name="T">The return result for the experiment.</typeparam>
/// <typeparam name="TClean">The clean result for publishing.</typeparam>
public interface IExperiment<T, TClean> : IExperiment<T>

which means the compiler won't allow IExperiment<TControl, TCandidate>

Looking into solutions, happy to hear anyone's thoughts?

One initial thing that comes to mind, is forcing anyone who wants this feature to use TClean as well, but it feels a bit heavy handed