NightOwl888 commented on a change in pull request #347:
URL: https://github.com/apache/lucenenet/pull/347#discussion_r494893175
##########
File path: src/Lucene.Net/Support/Diagnostics/Debugging.cs
##########
@@ -26,6 +26,9 @@ namespace Lucene.Net.Diagnostics
/// </summary>
internal static class Debugging
{
+ // LUCENENET specific - use a lazy wrapper around SystemProperties to
avoid the repeated cost of getting the value
+ private static Lazy<bool> _assertsEnabled = new Lazy<bool>(() =>
SystemProperties.GetPropertyAsBoolean("assert", false));
Review comment:
Actually, some people used `_assertsEnabled` and other people used
`assertsEnabled` for private/internal member variables throughout the project.
During the API sweep, they were not normalized as the public/protected
variables were, primarily because it isn't as high of a priority.
> The main focus was to eliminate all of the `PascalCase` member variables,
which caused a lot of debugging grief - in some cases a field got called where
a property should have or vice versa because of errors during porting. Not to
mention all of the extra mental anguish trying to work out what is a property
and what is a field while debugging. Not sure why that convention was used, as
it isn't a common convention in either Java or .NET. But now all of those
fields have been renamed back to `camelCase`.
Currently, there are [tests that use
Reflection](https://github.com/apache/lucenenet/blob/ece6bea0a3c98961a77d7060b5615fccefabe725/src/Lucene.Net.Tests.TestFramework/Support/TestApiConsistency.cs)
to locate naming and other coding violations, but we probably should aim to
use a tool such as Resharper or CodeAnalysis as a replacement at some point.
The tool we use should have the ability to make customizations so we can use it
to catch Java to .NET conversion errors as well (i.e. `size()` should be
converted to `Count` or `Length`, not `Size()` or `Size`).
One convention that is a bit odd and is a special case because of the
tendency to make a public `getSomething()` method in Java along with a
protected `something` member variable. If we converted this to public
`Something` property and protected `something` member variable, we end up with
a CLS compliance conflict. As a result, all protected members (whether there
was a conflicting property or not) were prefixed with `m_`, since prefixing
with `_` is also not CLS compliant. Of course, in .NET it really doesn't make
much sense to expose the member variable as protected if there is already a
property to access or set it, so we probably could change this at some point as
well.
> I started using Resharper recently after I discovered that [JetBrains has
a free license program for
Apache](https://blog.jetbrains.com/blog/2019/05/30/jetbrains-supports-the-apache-software-foundation/).
I used it to change all of the single-lined properties to use expression
syntax, but haven't dug down into the settings files yet. No objections if you
want to add one as long as the rules don't conflict with our API tests.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
