[Jderobot-admin] EasyProxy
Victor Arribas
v.arribas.urjc en gmail.com
Mar Dic 29 14:29:17 CET 2015
*Related to #269*
I prefer to write it here (in private) because do it publicly do not make
sense.
Context:
I saw some pieces of code at mainstream, reviewing it I saw a lot of issues.
If you wish you can review code before futher reading.
-
https://github.com/RoboticsURJC/JdeRobot/tree/master/src/stable/libs/jderobotHandlers
-
https://github.com/RoboticsURJC/JdeRobot/commit/f96c0185761374cf97e3cbf48b50d172f7c04bfa
I redo it because it was just ease to do. You can see it at
https://github.com/RoboticsURJC/JdeRobot/pull/269
This email completes last reply at github.
*Why jderobotHandlers is simply bad constructed*:
1. you created a class+getter. Are you kidding us? This is C++ not Java,
please use a function
2. you create a cpp and compile the library for this "feature". Templates
stop working after compile; even more when template domain is not closed.
This is a huge pitfall. A simple header without library supporting it is
the way to do it, see my approach to understand it, or just see how boost
works.
3. I should review replayer before say anything. By the way, if
replayerController is a front end of replayer (am I wrong?) and therefore
it is a runtime dependency, just push this handle in replayer component to
be coherent adding also build dependency. And YES, use a header only
approach to avoid compile and eventually link a library for it. Just with
the correct include_directories() and install() it will work like a charm.
4. Since proxy is already a smart pointer, and therefore memory release is
guaranteed, you should not need other "smart" layer. Your class can use
copy constructor safety and therefore use "new" and a "pointer to" by
default is a misconception.
As summary, you created a .so and you may never do that because implies a
misconception and unnecesary overload.
Why it happens? because you do it too fast. So do not be worried about it!
it can happen.
By the way, I was too rude due 2 reasons:
1. A day I found 4 commits at mainstream. I was smart enough and skip
them. Next day another fetch reveal me a few more. Some of them fixes and
even one that I already warned (#241, #256, #257)
2. jderobotHandlers has none documentation and was silently added with
above problems. Ask me to move my fork inside it seems a joke. (even more
after #248's reply)
------------ próxima parte ------------
Se ha borrado un adjunto en formato HTML...
URL: http://gsyc.escet.urjc.es/pipermail/jderobot-admin/attachments/20151229/ffd240b2/attachment.htm
More information about the Jderobot-admin
mailing list