SamboyCoding / Tomlet

Zero-Dependency, model-based TOML De/Serializer for .NET
MIT License
155 stars 29 forks source link

feat: add IEnumerable support for serialization and deserialization #35

Closed tcortega closed 8 months ago

tcortega commented 1 year ago

Adds basic IEnumerable support for deserialization and serialization. Treats IEnumerable de/serialization as if they were collection types.

I could not find a way to only commit what I've changed in the TestResources.resx but I hope this isn't a problem.

coveralls commented 1 year ago

Coverage Status

coverage: 91.65% (-0.8%) from 92.423% when pulling 6b32847baee1badf30ab6d5e25a9643e0b8a788b on tcortega:feat/ienumerable-support into 294ad2379e7be27a01f9ad0a4616c89a549bb65d on SamboyCoding:master.

SamboyCoding commented 1 year ago

I think - judging by the coverage report - that this breaks Dictionary serialization? Specifically referring to this report, because the check for Dictionary comes after the check for IEnumerable, and Dictionary inherits from IEnumerable. Not sure why the tests still pass though, that's a little strange.

tcortega commented 1 year ago

I think - judging by the coverage report - that this breaks Dictionary serialization? Specifically referring to this report, because the check for Dictionary comes after the check for IEnumerable, and Dictionary inherits from IEnumerable. Not sure why the tests still pass though, that's a little strange.

This happens indeed because the IDictionary<,> inherits from ICollection<T> which inherits from IEnumerable<T>. This means that it'll get the dictionaries enumerator and deserialize/serialize it, item by item, reading the KeyValuePairs.

Now the question that is left is, this approach works fine, however, it might be slower than the old serializer, I didn't benchmark it but it probably is. Do you want me to move the condition for the dictionary check to be above it?

SamboyCoding commented 1 year ago

I'm not only concerned about the speed, I'm also concerned that it'll likely serialize differently - I'd imagine it'd serialize as a table array with each table having a key and value field, instead of as a table. So yeah please move the check up.

levicki commented 8 months ago

@SamboyCoding Thanks for implementing this.