Change Reason.Create to enforce a Guid id

[TomasEkeli]

The current .Create accepts any string and crashes run-time. Let's use the compiler to remove a potential run-time error.

If we need it to be a Guid we should not allow strings, even if that makes the method slightly easier to use. It does so at the expense of making run-time errors more likely.

┆Issue is synchronized with this Asana task

[einari]

Great point @TomasEkeli - what if we went one further and actually use a ConceptAs<Guid> called ReasonId. We could then have have an implicit operator on that that parsed the string and actually have the code passing in a string and the parsing would happen implicitly and fail in the same way.

[TomasEkeli]

What I want to avoid is that code that looks fine and builds fine crashes at run-time

Example: Reason.Create("something not a guid", "A good reason", "description");

If we had a ReasonId : ConceptAs<Guid> with an implicit operator that does Guid.Parse we would still let this slip past the compiler and turn into a runtime exception.

Can we, perhaps, have strings as Ids instead of Guids?

[einari]

new Guid() or Guid.Parse() will not evaluate at build time, so you still have the same problem. It will only be seen at runtime no matter what you do for that.

If we want to evaluate this at build time, we could look into creating a Roslyn extension that does the check. We have a few Roslyn analyzers in the works already.

I'm not too keen on having it as a string, even though that would work in isolation. The reason is that we're looking at including rules, broken rules and reasons as part of the MetaModel and actually promoting Rules, BrokenRules and Reasons into Artifacts - which would then require them to be Guids.

[ghost]

➤ Einar Ingebrigtsen commented:

I will move this into Triage for prioritization for now and as a non-breaking change we'll add the ConceptAs<> for this, building on your Pull Request.

[einari]

Based off of this PR - I did some improvements in #317 - closing this as it is captured in that.