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 |
|
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
|
|
|
|
|
SurfaceCtrl, 3D viewer of Multiple file format and Surface class
By: Xemuth on Tue, 15 September 2020 15:46
|
|
|
Re: SurfaceCtrl, 3D viewer of Multiple file format and Surface class
By: koldo on Tue, 15 September 2020 17:07
|
|
|
Re: SurfaceCtrl, 3D viewer of Multiple file format and Surface class
By: Xemuth on Fri, 18 September 2020 23:15
|
|
|
Re: SurfaceCtrl, 3D viewer of Multiple file format and Surface class
By: Didier on Fri, 18 September 2020 09:55
|
|
|
Re: SurfaceCtrl, 3D viewer of Multiple file format and Surface class
By: koldo on Fri, 18 September 2020 18:52
|
|
|
Re: SurfaceCtrl, 3D viewer of Multiple file format and Surface class
By: Didier on Sat, 19 September 2020 12:46
|
|
|
Re: SurfaceCtrl, 3D viewer of Multiple file format and Surface class
By: Xemuth on Fri, 18 September 2020 23:10
|
|
|
Re: SurfaceCtrl, 3D viewer of Multiple file format and Surface class
By: koldo on Fri, 18 September 2020 19:12
|
|
|
Re: SurfaceCtrl, 3D viewer of Multiple file format and Surface class
By: Xemuth on Fri, 18 September 2020 22:57
|
|
|
Re: SurfaceCtrl, 3D viewer of Multiple file format and Surface class
By: Klugier on Fri, 18 September 2020 23:17
|
|
|
Re: SurfaceCtrl, 3D viewer of Multiple file format and Surface class
By: Xemuth on Sat, 19 September 2020 03:37
|
|
|
Re: SurfaceCtrl, 3D viewer of Multiple file format and Surface class
By: Klugier on Sun, 20 September 2020 00:11
|
|
|
Re: SurfaceCtrl, 3D viewer of Multiple file format and Surface class
By: Xemuth on Sun, 20 September 2020 16:45
|
|
|
Re: SurfaceCtrl, 3D viewer of Multiple file format and Surface class
By: Klugier on Thu, 24 September 2020 23:09
|
|
|
Re: SurfaceCtrl, 3D viewer of Multiple file format and Surface class
By: mirek on Wed, 17 February 2021 17:57
|
Goto Forum:
Current Time: Sun Nov 03 17:55:00 CET 2024
Total time taken to generate the page: 0.04044 seconds
|