MarimerLLC / csla

A home for your business logic in any .NET application.
https://cslanet.com
MIT License
1.27k stars 406 forks source link

Add new property required indicator to RegisterProperty #4351

Open StefanOssendorf opened 4 days ago

StefanOssendorf commented 4 days ago

Would it be useful to have a bool (or enum?) to declare "This property must be set (!= null) after any data portal operation". So that the framework would check all marked properties if they have value != null. If a violation is deteced it'll raise an RequiredPropertyNotSetException to provide immediate feedback for any developer.

Describe the solution you'd like Tbh I'm not a fan of using yet another bool but something like RegisterProperty<Foo>(nameof(MyFoo), isRequired: true); or a enum like RegisterProperty<Foo>(nameof(MyFoo), options: PropertyOptions.Required); where PropertyOptions is a flag so we can use it in the future for more settings?

Describe alternatives you've considered The developer is obligated to have some kind of guard method which must be added to all fetch, create, update, ... methods.

kant2002 commented 4 days ago

The main question what are the other options in the enum? It seems that passing enum is optimization for some future which may never come. Maybe you hav e some concrete ideas of what other enum fields would be

rockfordlhotka commented 4 days ago

We already have overloads that accept a flags enum to indicate lazy loading. My thought was to add a new flag to that existing enum - no extra overloads of RegisterProperty necessary.

rockfordlhotka commented 4 days ago

If we can detect, at runtime, that a property/field is required then we should be able to use that metadata all the way down the call chain into the set/load property methods to throw if they are provided a null value. Then within the call chain we can safely assume that any required property will never have a null value.

Such properties also can never be allowed to go to a default value, because default would be null, so they have to be initialized to something as part of the create/fetch operations.

StefanOssendorf commented 3 days ago

The main question what are the other options in the enum? It seems that passing enum is optimization for some future which may never come. Maybe you hav e some concrete ideas of what other enum fields would be

The main point was to have an easier time in the future to add new option values instead of adding more booleans or changing from a bool to an enum. But as rocky stated we already have that enum. I totally forgot that!

StefanOssendorf commented 3 days ago

If we can detect, at runtime, that a property/field is required then we should be able to use that metadata all the way down the call chain into the set/load property methods to throw if they are provided a null value. Then within the call chain we can safely assume that any required property will never have a null value.

Such properties also can never be allowed to go to a default value, because default would be null, so they have to be initialized to something as part of the create/fetch operations.

That is my intention. We have access (or can make it available) to the FieldManager during fetch, create, update, etc to check exactly that. Like we changed in #3776 to wait for non-busy.

There will only be these situations where these properties can (and will) be null

All other methods (update, execute, delete) will have these properties set.