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

[Serialization] More useful throw when missing parameterless ctor #2098

Merged
merged 5 commits into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -816,6 +816,25 @@ public void DeserializeWithRepeatedSubObjects()
Assert.Equal(22, family.Mother.Age);
}

[Fact]
public void ThrowWithoutEmptyCtor()
{
try
{
SerializeThenDeserialize(new ClassWithNonEmptyCtor(default));
Assert.Fail("An exception should have been thrown given that the class cannot be instanced because of the missing empty constructor ");
}
catch (Exception ex)
{
Assert.IsType<DefaultObjectFactory.InstanceCreationException>(ex.InnerException);
}
}

class ClassWithNonEmptyCtor
{
public ClassWithNonEmptyCtor(bool parameter) { }
}


[Fact]
public void DeserializeEmptyDocument()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,14 @@ public static Type GetDefaultImplementation(Type type)
return type;
}

/// <inheritdoc/>
public object Create(Type type)
{
type = GetDefaultImplementation(type);

// We can't instantiate primitive or arrays
// We can't instantiate primitives or arrays
if (PrimitiveDescriptor.IsPrimitive(type) || type.IsArray)
return null;
throw new InstanceCreationException($"Failed to create instance of type '{type}', wrong factory.");

if (type.GetConstructor(EmptyTypes) != null || type.IsValueType)
{
Expand All @@ -119,11 +120,12 @@ public object Create(Type type)
}
}

return null;
throw new InstanceCreationException($"Failed to create instance of type '{type}', type does not implement a parameterless constructor.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: a type has or doesn't have a parameterless constructor. It doesn't really implement it, the same way an interface or abstract method can be.

Suggested change
throw new InstanceCreationException($"Failed to create instance of type '{type}', type does not implement a parameterless constructor.");
throw new InstanceCreationException($"Failed to create instance of type '{type}', type does not have a parameterless constructor.");

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

}

public class InstanceCreationException : Exception
{
public InstanceCreationException(string message) : base(message) { }
public InstanceCreationException(string message, Exception innerException) : base(message, innerException) { }
}
}
Expand Down
6 changes: 3 additions & 3 deletions sources/core/Stride.Core.Yaml/Serialization/IObjectFactory.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2015 SharpYaml - Alexandre Mutel
// Copyright (c) 2015 SharpYaml - Alexandre Mutel
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to deal
Expand Down Expand Up @@ -56,8 +56,8 @@ namespace Stride.Core.Yaml.Serialization
public interface IObjectFactory
{
/// <summary>
/// Creates an instance of the specified type. Returns null if instance cannot be created.
/// Creates an instance of the specified type. Throws with an appropriate exception if the type cannot be created.
/// </summary>
object Create(Type type);
}
}
}