-
-
Notifications
You must be signed in to change notification settings - Fork 865
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Coding Style #1
Comments
Hi Bernard. You are perfect right. This structure grew into place like this. At the start is was easy and quick to develop right this. But now the engine has grown quite a bit and that did not make it faster to compile or really clean. I agree with the documentation in header files. I'll clean it up a bit later ok? I understand how the code fits together so it's easier for me to split up properly. Note for a general structure and idea, be sure to read the readme, this explains a few basic things. (I hate doxygen, on my previous job they managed to make the comments into an unreadable mess with it) But the code really needs some extra comments. (As for "inline" this is not always respected by GCC. See https://www.kernel.org/doc/local/inline.html The "vector" functions need to be in a header like this to be properly inlined, but the rest should be in cpp files) But first, I have to do some Cura GUI bugfixing. |
Totally refactored the code to fix the coding style. It's on the refactor branch right now: https://github.com/Ultimaker/CuraEngine/tree/refactoring |
First of all:
Great work! Seriously.
Second:
by having just h files you dramatically slow down compilation, as always everything needs to be build. You cannot have .o files in between, which thanks to Makefile would not need recompilation.
From my knowledge, just declaring a function inline is sufficient as optimization, and should be respected.
Also: it makes the code much harder to read, which I just tried. Its quite difficult to get a general glimpse of what is going on, i.e. by looking at the class structure.
Also, the .h usually usually also contains documentation e.g. for doxygen.
I can only estimate how busy you are at the moment, and so am i, but I am willing to give it a shot in converting it to a normal c++ structure.
The text was updated successfully, but these errors were encountered: