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

Coerce output in RuleContext.AddOutValue #3925

Merged

Conversation

rrrooommmaaa
Copy link
Contributor

Close #3251

@StefanOssendorf
Copy link
Contributor

Thank you for your contribution.
I'm not sure if the implementation is what we want. @rockfordlhotka shoud make a decision here.
My "problem" is: With the new implemention you can't set a string property to null because the CSLA default is string.Empty.
This is a breaking change in current behavior and I don't know how many people rely on that.
I'm personally okay with this change.

Copy link
Contributor

@StefanOssendorf StefanOssendorf left a comment

Choose a reason for hiding this comment

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

As marker to wait for response from @rockfordlhotka

@rrrooommmaaa
Copy link
Contributor Author

@StefanOssendorf I've got the same conversion from null to String.Empty when trying to write null value directly to a string property because of

if (typeof(P) == typeof(string) && newValue == null)
newValue = Utilities.CoerceValue<P>(typeof(string), null, string.Empty);

@StefanOssendorf
Copy link
Contributor

@StefanOssendorf I've got the same conversion from null to String.Empty when trying to write null value directly to a string property because of

if (typeof(P) == typeof(string) && newValue == null)
newValue = Utilities.CoerceValue<P>(typeof(string), null, string.Empty);

That may be the case. But currently a rule can set a string to null with an out value. I just tested that with 8.1.0.

@rockfordlhotka
Copy link
Member

The reason that, in general, CSLA tries to ensure that a string property is empty instead of null, is that (iirc) Windows Forms data binding can't handle null in some cases.

If someone is using Windows Forms and they created a rule that set a string to null, that would probably cause data binding to fail in their app. I don't think anyone else would notice, because I don't think any other UI frameworks have this limitation.

All that said, this is a breaking change - but one that has no real meaning, since CSLA string properties generally convert null into empty already.

Someday maybe we'll change everything to support null string property values, but that day is not today.

I think this change is fine.

@rrrooommmaaa
Copy link
Contributor Author

@StefanOssendorf Can we merge then?

@StefanOssendorf
Copy link
Contributor

Alrighty. LGTM then :-)

@StefanOssendorf StefanOssendorf merged commit fa03c4f into MarimerLLC:main May 13, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support parameter conversion in RuleContext.AddOutputValue
3 participants