Project Code Cleanup - Improve Code Quality

[epsi1on]

EDIT:

Milestone 1

• BriefFiniteElementNet / BriefFiniteElementNet.Common
  • [ ] Remove obsolete code.
  • [ ] Fix not fully implemented classes that use Obsolete attribute.
  • [ ] Inspect Unreachable code detected warnings and fix or comment what to do.

• [ ] Add .editorconfig to maintain a consistent coding style.

Milestone 2

• BriefFiniteElementNet / BriefFiniteElementNet.Common
  • [ ] Add unit tests for public (and internal) classes.
  • [ ] Add XML comments to public classes.
  • [ ] Add more XML comments (internal or even private classes).
• BriefFiniteElementNet.Validation
  • [ ] Remove BriefFiniteElementNet.Controls dependency

Milestone 3

• BriefFiniteElementNet / BriefFiniteElementNet.Common
  • [ ] fix TODO items
• BriefFiniteElementNet.Validation
  • [ ] fix TODO items ======================

discussion continued from comment number 673732028 of #36 (link)

1. I think having a good coverage is crucial to ensure that people contributing code do not unintentionally break anything. OpenCover reports the following code coverage, which is (expectedly) low: Line coverage: 13.4% (2266 of 16832) Branch coverage: 13.0% ( 494 of 3788)

Yes i also think that unit tests are not enough yet, only some element helpers have unit tests, others do not. So is it necessary to increase the unit tests? i think there is need for it

2. When building the solution, Visual Studio reports 345 warnings, some of them warning about obsolete code usage, which should easily be fixed, but also many obsolete warnings, telling a feature is incomplete/not working. Such code should not be distributed via nuget (meaning not be part of the main project or master branch).

I've used [Obsolete()] attribute for both deprecated codes and new codes which are under development and not tested yet. As you stated, I'm also waiting to get an stable version then publish that version in nuget.org.

3. Building the solution with enabled generation of documentation, 1185 warnings are reported. So there's also a massive lack of documentation, which might discourage people to contribute code (because they don't understand the intention of the code that's already there).

You mean XML documentation for methods and fields and property ?

Thanks

[wo80]

EDIT: removed task list.

[wo80]

You can either remove Milestone 3 or add some specific tasks (one addition - with rather low priority - could be to inspect the TODOs scattered across the projects).

I think you should put the heading Plan towards BriefFiniteElementNet version 2 and a nuget release back in place (or change the title of the issue). All the tasks should be addressed before publishing a final nuget package. Of course you can publish prerelease packages - as you might have noticed, I added <Version>2.0.0-pre</Version> when converting the project files. The pre means nuget will handle it as a prerelease.

[epsi1on]

Maybe need to make a new project with title Plan towards BriefFiniteElementNet version 2 and a nuget release? First we should address stuff that need to be done before release nuget package...

[wo80]

• Having a project to track the progress sounds like a good idea.
• You can publish a prerelease/alpha/beta package at any point. The final package should address all tasks that we define in this issue. It's up to you to decide what the final release should look like, so feel free to add or remove tasks.
• Each milestone should be finished before working on the next milestone starts. Obviously, you don't want to write tests or documentation for code that will be deprecated.
• For each task defined, details could be discussed in a separate issue.

[epsi1on]

I rather to move obsolete classes into another namespace, and remove them in next version, what you think about it? For integration tests, you mean it should be automatically tested by visual studio? currently it is a console application...

[epsi1on]

https://github.com/BriefFiniteElementNet/BriefFiniteElement.Net/projects/4

[wo80]

I rather to move obsolete classes into another namespace, and remove them in next version, what you think about it?

If you think that the obsolete code is still widely used, that would be a fair compromise. Alternatively, you could add another project BriefFiniteElementNet.Compatibility, which could even be distributed via nuget, if anybody feels the need.

For integration tests, you mean it should be automatically tested by visual studio? currently it is a console application...

If I'm correct, at the moment it's a loose collection of tests, which must be run manually. Just add a class TestRunner (or the like) which calls all those tests and gives a hint if anything doesn't work as expected. I will start working on the Matrix class, once we have this.

[rubsy92]

Hi,

I'm going through the todo-list and I noticed item "complete TriangleElement codes that support all 3 types of loads.": https://github.com/BriefFiniteElementNet/BriefFiniteElement.Net/projects/4#card-44141482. What do you mean with all 3 types of loads?

[epsi1on]

There are 3 type of elemental loads, Uniform, Concentrated and PartialNonUniform. I think currently only uniform load is supported by triangle element. At least IElementHelper.GetLocalEquivalentNodalLoads() method should be implemented for both DkqHelper and CstHelper and the code should support ConcentratedLoad and PartialNonUniformLoad .