[simspark-devel] Re: communication code progress

[Markus R. <rol...@un...>]

Hi,

Hesham wrote:
> Thank you so much for your fast and complete review. My code had a lot 
> of coding style mistakes, if I know this I didn't send it :-)

I think your code was actually quite ok. It's just that every project 
uses it's own style of coding in order to make life easier for the 
developers as the need to read each other's code. Otherwise the codebase 
would result in a mixture of different styles. The only trouble is that 
most projects use their very own style guide. So sending in you're code 
for review is a good way to adapt to it ;-)

>     - What is the meaning of the 'ifText' member? Please add a comment to
>     sayeffector.h explaining it
> 
>     - All member variables should start with a small 'm' for
>     'member'. Therefore the ifText member should be renamed 'mIfText'
> 
>  
> But I didn't write it. It seems it's a variable that sexpmonitor uses 
> that. But I didn't change it because I'm not sure about it.

Ok I see. It seems that Irene Markelic committed that code... (according 
to 'cvs annotate sayeffector.h') But now you're our expert for the 
sayeffector ;-)

Do you understand how ifText is used in the sayeffector? Can you come up 
with a better name for it? Or can we safely remove this code?

I looked through the changed version of you're code and I think it looks 
very good now. If Joschka and Oliver are Ok with it I think we can 
commit it.

I think there is just one thing missing: In order to use the say 
effector with the soccer simulation you had to modify the 
rcssserver3d.rb (app/simulator/rcssserver3d.rb, function addagent) to 
install the effector. I think you missed to include this change in the 
file you sent.

regards,
Markus