Csla.Rules.BusinessRules.GetAllBrokenRules - BrokenRules is null

[ajj7060]

I haven't had a need to use the Csla.Rules.BusinessRules.GetAllBrokenRules method before now, and I hit something unexpected. I was attempting to flatten the broke rule descriptions into a single list, and my code was giving me a NullReferenceException. My object graph is a Root containing a List containing Child instances. The exception was from this code:

Csla.Rules.BusinessRules
    .GetAllBrokenRules(requestQueue, true)
    .Where(x => x.BrokenRules != null && x.BrokenRules.Any()) // This is my workaround
    .SelectMany(x => x.BrokenRules)
    .Select(x => x.Description)

The BrokenRules seems to be null for the BusinessListBase instance, and was causing SelectMany to blow up. Its not difficult to work around, but it seems odd that a collection property returns null instead of an empty collection.

Is this a bug, or is there a reason null is returned here instead of an empty collection?

[rockfordlhotka]

I'm going to be cliché and say it is a 'feature' :grinning:

Looking at the code in the BusinessRules class it appears that when a collection has no children with broken rules it isn't added to the list of return values.

[ajj7060]

May I request that this be reclassified as a bug? :-) I think in .Net there's a pretty strong expectation that collection properties never return null.

It does appear though that the Collection's BrokenRules property is null even when it has a child with BrokenRules.

[jonnybee]

A list does NOT aggregate the broken rules of it's children.

GetAllBrokenRules will return the object tree where each object has it's own set of BrokenRules. An object may be a list or a editable object or a readonly object.

So what Rocky said is that a list does not have a concept of ValidationRules and hence by design the BrokenRules property is null (and this also saves space in serialization).

[rockfordlhotka]

I suppose we could return an empty list instead of null - would that make things simpler?

[ajj7060]

@jonnybee I'm not saying a list should aggregate the broken rules of its children. I'm just saying for collection BrokenRuleNode (I think that's the class), BrokenRules should return an empty collection, so that other's aren't surprised with a NullReferenceException when they try to flatten the broken rules for the graph, like I was.

@rockfordlhotka Yes, I think it would. It took me a bit of time (not a lot, but it wasn't immediately obvious either) to track down where the NRE was coming from in my sample code.

[rockfordlhotka]

I think we probably could have GetAllBrokenRules create/return an empty list - that way we wouldn't add anything to the serialization payload or even memory footprint - we'd just provide this value to make navigating the tree easier.

I want to get @jonnybee 's thoughts before proceeding though, as I sometimes miss obvious problems that he catches :smile:

[jonnybee]

I have no issues with this. I can't think of anywhere that would be a problem.

[rockfordlhotka]

Sounds good. @ajj3085, do you want to take a run at this change and submit a pull request?

[ajj7060]

@rockfordlhotka I'll try working on this tomorrow, I've never used git before (except the issues portion :-) ).

[jonnybee]

Hi,

It is already implemented and in PR (pull request) for merge into master.

[ajj7060]

@jonnybee Oh ok, thanks!

[rockfordlhotka]

:smile: