Overview
Examples
Screenshots
Comparisons
Applications
Download
Documentation
Tutorials
Bazaar
Status & Roadmap
FAQ
Authors & License
Forums
Funding Ultimate++
Search on this site
Search in forums












SourceForge.net Logo
Home » Developing U++ » UppHub » SurfaceCtrl, 3D viewer of Multiple file format and Surface class (Visualise in 3D many 3D file format object and even all Surface object !)
Re: SurfaceCtrl, 3D viewer of Multiple file format and Surface class [message #54835 is a reply to message #54832] Sun, 20 September 2020 16:45 Go to previous messageGo to previous message
Xemuth is currently offline  Xemuth
Messages: 387
Registered: August 2018
Location: France
Senior Member
Hello Klugier, Thanks for this code analysis, all the think you said help me to become better ! Thanks a lot !

Klugier wrote on Sun, 20 September 2020 00:11

- In Definition.h "#define GL GLCtrl" this is not needed explicit GLCtrl is fine and additional define can be pass to the user.
- "enum Camera_Movement {CM_FORWARD,CM_BACKWARD,CM_LEFT,CM_RIGHT};" modernized to enum class you will much better type control.
- Camera_Movement CamerMovementDirection shold be enough. I think _ is not needed in public API.

fixed / applied

Klugier wrote on Sun, 20 September 2020 00:11

- noexcept - why it is used in so many places. I think it obscures readability and can be ommited. I do not see noexcept such common in uppsrc.

I have read in a book about advanced C++ that compiler will remove all stuff about exception when compiling a function if it is marked as noexcept, so it optimize the code to run faster. May the result is not high enought to obstruct readability ?

Klugier wrote on Sun, 20 September 2020 00:11

- "void MoveAllSelectedObjects(glm::vec3 move)noexcept; //Move all selected object" - this comment is redurdant to the method signature (it occures in many places). If you want documentation just use Topic++.

indeed
Klugier wrote on Sun, 20 September 2020 00:11

- Long functions are implemented in the header file. If function is not one liner then it should be moved to .cpp file.

What about header only file ? should I create A cpp file even if in my header only class there is somethings like 3 function which take more than one line ?


Klugier wrote on Sun, 20 September 2020 00:11

- Shader managment can be placed to separate package. I know we have some shader related stuff in GLDraw, but would be nice if we can unify this. Please check the code behind it

I will take a look, since modern OpenGL use (90% of time (because fixed pipeline is strongly deprecated)) shaders are the good way to work .Maybe it would be good to create a GLShader package which goes with GLCtrl package ?
Maybe I can developpe something in that way ?

Klugier wrote on Sun, 20 September 2020 00:11

- public method/members should go first - then you should place private variables. The first thing I want to examin is the class public API. I do not want to analyze implementation

Applied

Klugier wrote on Sun, 20 September 2020 00:11

- French comments "//Gestion of alpha" - anyway in the modern programming comments should be avoided - your code should be clear enough to exist without comments. Of course, sometimes this rule may be deviated from, but in very rare cases.

I guess you are right but atm my code is full of comments so, to remove them I must need documentation but it take time. I will try to remove them in futurs update

Klugier wrote on Sun, 20 September 2020 00:11

- Magic numbers - what is 20000. For more info please visit https://clang.llvm.org/extra/clang-tidy/checks/readability-m agic-numbers.html. Optimally you should define constexpr variables.

Indeed, the readability is better, I will update my code in this sense

 
Read Message
Read Message
Read Message
Read Message icon14.gif
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Previous Topic: How to move packages from bazaar to github repo and UppHub
Next Topic: Non core packages shouldn't be under ultimatepp umbrella organization on GitHub
Goto Forum:
  


Current Time: Sun Nov 03 17:55:00 CET 2024

Total time taken to generate the page: 0.04044 seconds