code cleanup

[MovGP0]

did the following cleanup:

• enabled nullable checks
• added missing private/internal keywords
• fixed autoproperties
• fixed nullable warnings
• removed unused namespace imports
• cleaned up namespace imports
• removed redundant type declarations
• fixed some inheritdoc attributes where there is no doc to interit from
• added .idea folder to gitignore

Note: additional cleanup is required to fix all the nullable issues

[wo80]

Thanks, @MovGP0

Massive pull request, a lot of stuff going on here! Some stuff I'd be glad to merge, other that I don't like too much. Was this done by some kind of automated code correction?

From skipping over parts of the code, here are the things I dislike (basically my personal coding style preferences):

• The use of is not null instead of != null
• Using var for basically everything. Especially on types like int, float and double I like to be explicit. And for (var i = 0; ...) instead of for (int i = 0; ...) just doesn't feel right :-)
• I do like the nullable .NET feature, but having >1k warnings about it is rather annoying. Who's gonna fix this? Enabling the feature for new projects makes sense, but not so much for old projects.

I'll have a second, closer look at this PR in the next days.

[MovGP0]

The use of is not null instead of != null

Those have a different meaning:

• is not null checks if the variable is null
• != null uses the overload of the != operator to check against null, but it's not ensured by the runtime that this is correct.

Therefore, the is not null operator is recommended.

Using var for basically everything.

The use of var is recommended because it makes refactorings easier. The type is still shown by modern IDEs, so no information is lost. Just might take some time to get used to it.

having >1k warnings about it is rather annoying

Nullability-bugs are the most common form of bugs in code. Unfortunately, the nullability issues won't get fixed without those warnings.

[wo80]

Therefore, the is not null operator is recommended.

Agree.

The use of var is recommended because it makes refactorings easier. The type is still shown by modern IDEs, so no information is lost. Just might take some time to get used to it.

I do use var all the time, but not on the value types mentioned. Especially for numeric types, writing the type explicitly makes the code easier/faster to comprehend (at least for me).

---

I had a detailed look at all the code changes. I hope we agree, that opening such a large pull request without prior discussion is not optimal. Having coding style changes that we partly disagree on as main content, I think I'm not going to merge at this point. I'm not sure if it makes sense trying to fix these issues, but here are few more things I noticed:

• Why move enums from Enums.cs to Enums subdirectory?
• The use of using without fully qualified namespace (I know it works, but I prefer writing the full namespace).
• While fixing inheritdoc, why not provide proper documentation?
• Why specify internal for classes whilst not necessary?
• Applying nullable scheme where it doesn't make sense (Vertex? in TriangleWriter.WriteElements()).
• Inconsistent use of var with struct types (default vs default(Otri) in many places).
• Adding if (edge is null) continue; in DcelMesh.IsConsistent() changes semantics of the method (should return false or throw to match previous behaviour).
• Removing a method that was commented out in BoundedVoronoi (I actually forgot it was there, so thanks for the reminder).

As mentioned in my first comment, I would like to apply parts of the changes. Let me know what you think would be the best way to proceed, so that your contribution doesn't go unnoticed.