search

apache / logging-log4net

Apache Log4net is a versatile, feature-rich, efficient logging API and backend for .NET
https://logging.apache.org/log4net
Apache License 2.0
859 stars 327 forks source link

Nullability and modernization updates, part 3 #126

Closed erikmav closed 7 months ago

erikmav commented 7 months ago

For #124

erikmav commented 7 months ago

@FreeAndNil Important to closely review ReadOnlyPropertiesDictionary and PropertiesDictionary. I selected the most backward compatible option, which is to coerce the this[] getter to avoid throwing KeyNotFoundException like Dictionary<> would (including a new test for the behavior). Also note that to avoid a double-indirection it would be ideal to change both to structs.

FreeAndNil commented 7 months ago

Thanks. I will review on Monday.

FreeAndNil commented 7 months ago

Also note that to avoid a double-indirection it would be ideal to change both to structs.

@erikmav can you explain how you would implement this? Maybe as separate issue?

erikmav commented 7 months ago

Could be done under another issue. The change is to change from class to struct (or readonly struct, depending) for ReadOnlyPropertiesDictionary, then derive PropertiesDictionary from that as a second struct. I wouldn't do it in this PR either way, it's mainly an optimization.