BHoM / BHoM/BHoM_Engine

Serializer_Engine: serialization of `Func<>` and testing against C# base types

Open
#3,048 0 comments 0 reactions 4 assignees Assigned to @alelom View on GitHub
type:feature type:question
Dominant language
C#
Stars
30
Forks
13
Avg merge
7d 10h
Merged PRs (30d)
5

Description

#### Description:

As mentioned in:
https://github.com/BHoM/BHoM_Engine/pull/3042#issuecomment-1551531495

@pawelbaran identified an issue on the serialization of the `Diff` object: the serializer throws an error when serializing a `Diff` object created via the RevitDiffing workflow.

This is because the `Diff` object owns a `DiffingConfig` property, which itself owns a property of type `Func<>`. (_This `Func` property is required to obtain a dependency decoupling when passing diffing configuration from downstream repo to the base Diffing_Engine_). The serializer is unable to serialize/deserialize or ignore the `Func`, and it throws an error when encounters one, but the rest of the `Diff` object is serialized/deserialized correctly. This behaviour is consistent to what we used to have in the earlier version, but it was not discovered before for two reasons:
  A) because the exception was "swallowed" in UIs and not displayed (quoting @pawelbaran). In appearance, the Diff serialized just fine (with the exception of the `Func` property remaining null).
  B) Because the Func object is needed for the back-end and not manually created/queried in UIs, so users do not realize it is null when deserializing a Diff.

The last serializer refactoring removed the error "swallowing" (A) so this error is now surfacing when serializing/deserializing `Diff`s that have the `Func` property populated. This notably happens only in the RevitDiffing workflow.

Points to be discussed:
1. A solution would be to add support for `Func<>` in the serializer. Do we want to add support for it? How complex is it?

If not, we'd need to ask ourselves what other base C# types may be unsupported by serializer_engine. This issue demonstrates that we haven't got full test coverage against all C# base types when it comes to serializer engine; there may be more base C# types that the serializer engine _implicitly_ does not support. If we deny support for `Func`, we'd need to add test coverage against `Func` and more base C# types (a _big enough_ subset of them). Additionally, we'd need to add new compliance rules to check that neither `Func` nor any other serializer_engine-unsupported base C# type is not used across BHoM.
2. An alternative solution (workaround) would be to modify the DiffingConfig's `Func<>` property to `MethodBase`. This would make serializer_engine work "out-of-the-box" with no error against the `Diff` object, as tested by @pawelbaran, but it comes with lesser type-strong checking on the arguments.
3. (minor point) The current RevitDiffing wraps the function passed into the `Func<>` property into an anonymous lambda ([here](https://github.com/BHoM/Revit_Toolkit/blob/ae9da86c8972d9057c65dd0a438fa00fc834673b/Revit_Engine/Compute/RevitDiffing.cs#L223-L227)). This is unnecessary; anonymous functions cannot be serialized. That can be easily fixed by passing the function without wrapping. However, in order for the serializer to not throw errors, we still would need point 1.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.