Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deserializing to a custom model containing an IEnumerable<sometype> #75

Open
levicki opened this issue Nov 6, 2023 · 5 comments
Open
Labels
enhancement New feature or request PR welcome User contribution/PR is welcome

Comments

@levicki
Copy link

levicki commented Nov 6, 2023

Please consider this code sample:

public class TestItem
{
    public string Name { get; set; }
    public int ID { get; set; }
}

public class Test
{
    public IEnumerable<TestItem> Items { get; set; }
}

Test t = Toml.ToModel<Test>(File.ReadAllText("test.toml"));

And this TOML file:

[[Test.Items]]
Name = "Foo"
ID = 1

[[Test.Items]]
Name = "Bar"
ID = 2

Should this be possible to deserialize or not?

Currently it doesn't work (the error I get is error : Unexpected error when creating object for property Items on object type TestNamespace.Test. Reason: Cannot create an instance of an interface. which is expected because interfaces cannot be instantiated).

The reason why I use IEnumerable instead of say List is to keep datatypes as generic as possible (they are used in an interface definition which shouldn't care what the actual type is as long as it has IEnumerable implementation).

Is there some workaround for this without changing the datatype?

Note that Newtonsoft.Json has no problem deserializing to those IEnumerable properties.

@xoofx
Copy link
Owner

xoofx commented Nov 6, 2023

As a workaround, you can plug your own CreateInstance in the options here (The default being defined here that you can refer to from TomlModelOptions.DefaultCreateInstance)

@xoofx xoofx added enhancement New feature or request PR welcome User contribution/PR is welcome labels Nov 6, 2023
@levicki
Copy link
Author

levicki commented Nov 9, 2023

@xoofx Did you have something like this in mind?

private static object MyCreateInstance(Type type, ObjectKind kind)
{
	if (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(IEnumerable<>)) {
		Type inner = type.GetGenericArguments()[0];
		return Activator.CreateInstance(typeof(List<>).MakeGenericType(inner));
	}

	if (type == typeof(object)) {
		switch (kind) {
			case ObjectKind.Table:
			case ObjectKind.InlineTable:
				return new TomlTable(kind == ObjectKind.InlineTable);
			case ObjectKind.TableArray:
				return new TomlTableArray();
		default:
			Debug.Assert(kind == ObjectKind.Array);
			return new TomlArray();
		}
	}

	return Activator.CreateInstance(type) ?? throw new InvalidOperationException($"Failed to create an instance of type '{type.FullName}'");
}

It works well enough for my use case, I am just not sure whether List<T> is the best container to use since there's no way to check the number of elements in advance, and whether any additional type / kind checks might be needed (you probably have full battery of deserialization tests that could be run to check if this doesn't break something else).

@xoofx
Copy link
Owner

xoofx commented Nov 9, 2023

@xoofx Did you have something like this in mind?

Yep ☺️

I am just not sure whether List is the best container to use since there's no way to check the number of elements in advance

That's ok, I think that using List<T> is better than anything else for such a case.

PR welcome, there are currently not so many tests, so you will have likely to add one to cover this specific case.

@GSPP
Copy link

GSPP commented Aug 2, 2024

I had pretty much this issue, using an array as the property type:

Old:

public TestItem[] Items { get; set; } //Initialized to null

A workaround is to provide a non-null List:

public List<TestItem> Items { get; set; } = new();

@levicki
Copy link
Author

levicki commented Aug 2, 2024

I had pretty much this issue, using an array as the property type:

Old:

public TestItem[] Items { get; set; } //Initialized to null

A workaround is to provide a non-null List:

public List<TestItem> Items { get; set; } = new();

That's not a good idea — when writing interfaces you want to use as generic as possible types in the interface definition (hence the IEnumerable in my example) and leave it to the implementation to pick which container to use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request PR welcome User contribution/PR is welcome
Projects
None yet
Development

No branches or pull requests

3 participants