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

Consider whether lists can be initialized to empty instead of null #4266

Open
rockfordlhotka opened this issue Oct 22, 2024 · 3 comments
Open

Comments

@rockfordlhotka
Copy link
Member

          > > Another question: Any good reason why this lists can't just be initialized and hence empty if not neede and/or used? Makes the nullable stuff a lot easier and even more intuitive to use. Currently accessing `DirtyProperties` in a rule would return `null` instead of an empty list...

private readonly LazySingleton<Dictionary<IPropertyInfo, object>> _outputPropertyValues;
/// <summary>
/// Gets a dictionary containing copies of property values that
/// should be updated in the target object.
/// </summary>
[EditorBrowsable(EditorBrowsableState.Never)]
public Dictionary<IPropertyInfo, object> OutputPropertyValues
{
get
{
if (!_outputPropertyValues.IsValueCreated)
return null;
return _outputPropertyValues.Value;
}
}

private LazySingleton<List<IPropertyInfo>> _dirtyProperties;
/// <summary>
/// Gets a list of dirty properties (value was updated).
/// </summary>
/// <value>
/// The dirty properties.
/// </value>
[EditorBrowsable(EditorBrowsableState.Never)]
public List<IPropertyInfo> DirtyProperties
{
get
{
if (!_dirtyProperties.IsValueCreated)
return null;
return _dirtyProperties.Value;
}
}

CSLA often doesn't initialize lists of types that might be serialized, because it is much smaller to serialize null than it is to serialize an empty list. This is because the serializer doesn't need to flow the list's type information for null, but it does need to flow the type information for an empty list so the empty list can be deserialized on the other end of the process.

For MobileFormatter, yes, you need to serialize type information for a collection type so you know what to create on the other side. But that isn't a strict requirement for CSLA serialization. For example, my source generator serializer doesn't require this, because it knows at compile-time what the underlying type is for the field, and can new it up how it sees fit. For collections like List<T>, I don't special-case that one, but it's trivial to add a custom implementation for that in my generator through configuration.

Also, I do like empty collections over null in general overall. Then I don't need to wonder, "is this a null list or not?" everywhere in my code. If the performance implications of a null list over an empty one is significant during serialization, then....maybe keep them null, but if it's essentially negligible, then I'd much rather see an empty list.

Originally posted by @JasonBock in #1233 (comment)

@StefanOssendorf
Copy link
Contributor

I agree. I find it really weird to have a list null instead of just empty. What exactly is the difference in use?
If we want to keep/improve (de)serialization speed, we can special-case empty lists etc. so null is transported and we just re-initialize it on the other side.

@JasonBock
Copy link
Contributor

That's a good point. If a list is empty, serialize null, and then deserialize to an empty list.

I would really like to see perf data (using something like Benchmark.NET) to see what the perf benefit is to using null for serialization over an empty list. My guess is that it's negligible and doesn't outweigh the benefits of being able to reason about you code without worrying about null values. But the only way to know for sure is to measure and get numbers :)

@rockfordlhotka
Copy link
Member Author

Null and empty aren't the same thing, so I don’t think we can translate arbitrarily. I kind of suspect that would affect lazy loading, though that'll require some exploration.

It isn't the perfect of serialization that's in question, it is the size of the payload. And that may be incidental, so metrics need to be established.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants