logging-log4net-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jonas.Ba...@rohde-schwarz.com
Subject String Equality Comparison, Broken Tests and .NET-1.x
Date Mon, 22 Aug 2016 13:55:45 GMT
Hi,

A recent commit [1] changed, among other things, some string equality 
comparisons from `SomeComparer.Compare(a, "B", IgnoreCase) == 0` to 
`a.ToUpperInvariant() == "B"`, see also [2].
Unfortunately, this doesn't work if `a` is allowed to be null. Currently a 
lot of log4net.Tests are broken because of such a null reference exception 
in `NewLinePatternConverter.ActivateOptions` (apparently "%newline" is 
quite common in pattern layouts ;-).
For new code I tend to opt for `String.Equals(Option, "DOS", 
StringComparison.OrdinalIgnoreCase)` for a fast, case-insensitive 
comparison with fixed ASCII-only patterns, but static 
`String.Equals(String, String, StringComparison)` is not awailable on 
.NET-1.x [3].

So
-  `a.ToUpperInvariant() == "FOO" is not null-save and produces needless 
intermediate strings on the heap (and culture-aware operations are slower 
then nesessary for pure ASCII-stuff),
- `SomeComparer.Compare(a, "B", IgnoreCase) == 0` is hard to read, esp. 
when the compater is "long".
- we need .NET-1.x support, so all the goodness from [4] does not apply.

Should we create some helper in SystemInfo that provides null-aware, 
ordinal, casing-agnostic string equality comparison, with some #if's 
.NET-1.x? Who needs .NET-1.x? The current solution with `ToUpperInvariant` 
is also .NET-2.0...

[1]: r1756284 ".NET Core improvements by Peter Jas - closes #30"
[2]: https://github.com/apache/log4net/pull/16#discussion_r74683879
[3]: 
https://msdn.microsoft.com/en-us/library/t4411bks(v=vs.110).aspx#Anchor_4
[4]: https://msdn.microsoft.com/en-us/library/ms973919.aspx

Regards,
Jonas
Mime
View raw message