Thanks for your input Jonas. #30 is all around adding support for .net core
and so I believe there might follow a few fixing commits. Anyway, the
current status of .NET 1.x support is that we will drop it anytime soon. It
is so ancient that people might as well stay with the current state of the
framework. Its probably no longer worth the effort to maintain these old
branches. But if it is easy to keep support for it, I would not hesitate to
write a fix. Btw why not rewrite:

a.ToUpperInvariant()

to:

foo = "FOO"
a != null && foo == null || a.ToUpperInvariant() == foo

and there is still the option to #ifdef .net-1.x.

2016-08-22 15:55 GMT+02:00 <jonas.ba...@rohde-schwarz.com>:

> 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




-- 
Dominik Psenner

Reply via email to