search

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

Added a Func to allow the user to pass in result comparison logic #2

Closed RichardDalton closed 8 years ago

haacked commented 8 years ago

I like the way this is heading. We should also add a check if the objects implement IEquatable and use that if no comparer is set. I'd also like to have a default comparer for objects that does a shallow property comparison.

RichardDalton commented 8 years ago

I have now added the IEquatable check and refactored the comparison code to a separate method.

haacked commented 8 years ago

Thanks! I'll try and look over this during the weekend.

haacked commented 8 years ago

We should also add a check if the objects implement IEquatable and use that if no comparer is set.

So what I meant by this is we should check if the objects themselves implement IEquatable. I wasn't intending we add an equatable property.

Could you make just that change in a separate PR? I'd like to slowly iterate this to a better state. This PR is a lot to review and I'm not sure all of it is needed. I'd like to make one small change, try it out, and see what pain points we still have before adding more to it.

Make sense?

RichardDalton commented 8 years ago

Thanks for the review.

I wasn't intending we add an equatable property.

Sorry, I might be getting confused. Where did I add an equatable property?

haacked commented 8 years ago

Whoops, I meant IEqualityComparer https://github.com/Haacked/Scientist.net/pull/2/files#diff-766a1d63ded5bb40834df3dfad293be1R15

akamud commented 8 years ago

We were discussing some other alternatives to this implementation in #15.

Also, I'm planning on doing #16 as soon as this gets merged, I think this impacts directly on that issue.

RichardDalton commented 8 years ago

Replaced this with #24