language-ext icon indicating copy to clipboard operation
language-ext copied to clipboard

Fix fin enumerator

Open StefanBertels opened this issue 11 months ago • 2 comments

Fin<T> has wrong type in IEnumerable<>.

Side effect of the bug: XUnit will run into stack overflow if Assert.Equal() is used (see test).

Workaround: convert to Option<T> with e.g. myFin.ToOption()

StefanBertels avatar Sep 07 '23 13:09 StefanBertels

This probably needs a wider discussion. Because IEnumerable<> was only there to facilitate serialisation; if you look at Either<L, R>, you'll see that its IEnumerable<> bound-type is EitherData<L, R>. Again, this is because of the limitations of Json.NET - it was the only way I could reliably serialise without writing bespoke JsonConvertors (which I didn't want to do, because I didn't want to bring Json.NET into the Core).

I have however been investigating the idea of building bespoke LanguageExt.Serialisation.* libraries (where * would be Newtonsoft, System.Json, XmlDocument, Protobuf, etc. Which means there'd be no need for IEnumerable implementations on Option, Either, Fin, etc. Backwards compatibility could be maintained through some bespoke SelectMany operators, or just an expectation that conversion should be explicit, using AsEnumerable().

So, at the moment, your fix does fix an issue (honestly, I'm not sure what I was thinking when I wrote that!) - but it also makes it inconsistent with Either - probably, it should be IEnumerable<EitherData<Error, A>>. But, that whole line of reasoning will go away in about a month when I start building the new serialisation libraries!

Maybe just use Fin<A>.ToArray() temporarily, whilst I have a think about the best course of action.

For all: Please let me know if you have a strong opinion on this.

louthy avatar Sep 08 '23 10:09 louthy

This all makes sense to me. I found it very practical to use IEnumerable<T> resp. SelectMany to mix up different container types but I see that this isn't clean and Either isn't compatible to this anyway. Being explicit is a good thing here because then we can really see that we "leave" the monadic type. But it should remain really easy to use Option etc. just like a container, maybe similiar to being able to enumerate key-value pairs in e.g. Map.

I agree that IEnumerable<EitherData<Error, A>> would be a good fit. This would allow really mixing Fin<A> with Either<Error,A>. Should they be really the same?

Using list pattern matching might improve user code so maybe get them together? Maybe .AsEnumerable() and .Case can both return the same type/interface that both implements IEnumerable<> and supports list matching (if the original type should not support list matching directly), maybe slicing. Seq?

Regarding serialization: Yes. Especially good System.Text.Json support would be great. I dropped Json.NET some time ago, System.Text.Json has a much cleaner design. I suggest to allow some configuration via attributes for better compatibility. Another discussion maybe...

StefanBertels avatar Sep 08 '23 20:09 StefanBertels