Agreed! But we see here much more substance.
FC Code Quality
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
Be nice to others! Respect the FreeCAD code of conduct!
Re: FC Code Quality
A Sketcher Lecture with in-depth information is available in English, auf Deutsch, en français, en español.
Re: FC Code Quality
As someone who reviews PRs (including several of his), I believe that not only is @berniev acting in good faith, but he is willing to both do the grunt work of actually implementing his suggestions (he's not just complaining for the sake of complaining), and also work constructively to improve his own work. I believe we can all act in that spirit of cooperation -- his suggestion uptopic is not a "whitespace" change, or pure formatting, it's a move to objectively better code.
Of course right now is not a good time to make sweeping changes to code that is going to be touched by @realthunder's Toponaming code, simply because merging that branch is already going to be a tremendous amount of work for everyone involved and there's no call for us to go making it any harder when there are other places we can work. Focusing on improvements to the GUI code is a good idea, since that mostly avoids toponaming stuff, and there's a ton of work to be done there anyway.
It goes without saying that newly-written "Color" and "Position" classes probably aren't actually going to get merged, but in the sense that this is a "proof of concept" of the sort of improvement berniev has in mind I think it's a good step. Clearly real code is going to either use the Qt or Coin color and position classes, depending on its use -- that doesn't affect the underlying principle of the suggestion, which echos pretty much all modern coding advice. (On that note, though, I'd add a book to your reading list, berniev -- have a look at The Pragmatic Programmer by David Thomas and Andrew Hunt. In particular I think you may want to reconsider the sort of subclassing you demonstrate in your example file.)
I'd suggest that overall in the codebase there are a few major items to consider refactoring, if you are looking for maximum concentrated impact:
Of course right now is not a good time to make sweeping changes to code that is going to be touched by @realthunder's Toponaming code, simply because merging that branch is already going to be a tremendous amount of work for everyone involved and there's no call for us to go making it any harder when there are other places we can work. Focusing on improvements to the GUI code is a good idea, since that mostly avoids toponaming stuff, and there's a ton of work to be done there anyway.
It goes without saying that newly-written "Color" and "Position" classes probably aren't actually going to get merged, but in the sense that this is a "proof of concept" of the sort of improvement berniev has in mind I think it's a good step. Clearly real code is going to either use the Qt or Coin color and position classes, depending on its use -- that doesn't affect the underlying principle of the suggestion, which echos pretty much all modern coding advice. (On that note, though, I'd add a book to your reading list, berniev -- have a look at The Pragmatic Programmer by David Thomas and Andrew Hunt. In particular I think you may want to reconsider the sort of subclassing you demonstrate in your example file.)
I'd suggest that overall in the codebase there are a few major items to consider refactoring, if you are looking for maximum concentrated impact:
- Break up the megaclasses and megafiles
- Get rid of macros and replace with modern C++
- Modernize old C-style code
- Improve const-correctness
- Improve convoluted function signatures
Re: FC Code Quality
Thank you for the kind words.
This post is to demonstrate criticism of FC code is indeed justified. Maybe a little humility from time to time would not go astray. Many messengers get shot or bullied or dragged off-topic.
This post is to demonstrate criticism of FC code is indeed justified. Maybe a little humility from time to time would not go astray. Many messengers get shot or bullied or dragged off-topic.
Re: FC Code Quality
Feels humility there
It's hardly respectful.
Even being right -- if it could be something absolute -- doesn't give you all rights.