Sat 13 Mar 2010 06:58:50 PM UTC, comment #9:
The algorithms looks good to me.
Some implementation remarks:
- Do not update PrettyDebEx.cc this file is obsolete
and should be deleted. In fact the functions defined
here are not referenced anymore but I did forget to
delete them.
May be you can do it in 2 steps.
1 delete the file and commit, then go on with the
current patch.
- I'd rather use stdint types like uint32_t and
not "unsigned int", this is not a big deal
but it'll be more easy to maintain and less
error prone for 64bits types.
static const unsigned int versionMajor
-->
static const uint32_t versionMajor
- It would be better to avoid std::cerr usage
unless for preliminary debugging purpose.
If the error leads to an exception you could
attach the message as Exception reason using
setException, e.g:
answer->setException(e_RTIinternalError,"RTIA protocol Error")
As usual if you need to build a more complicated
message you can use a std::stringstream to build it
- May be the anonymous enum
enum {
CONNECTION_PRELUDE, //!< Before OPEN_CONNEXION
CONNECTION_READY,
CONNECTION_FIN //!< After CLOSE_CONNEXION
} _connection_state;
could be named and documented
/**
* Add meaning full explanation for this ...
*/
typedef enum ConnectionState {
CONNECTION_PRELUDE, //!< Before OPEN_CONNEXION
CONNECTION_READY,
CONNECTION_FIN //!< After CLOSE_CONNEXION
} ConnectionState_t
ConnectionState_t _connection_state;
I'm sorry if I appear to be picky that time, but I think
that since more people get involved in the code we should
try to do our best to enhance the readability of the code
for newcomers.
If you are OK with my remark please update and commit.
If you are not may be we can go for a second
patch round :-)
|