Code review comment for lp:~vijit.chauhan/beeseek/fix-603119

Revision history for this message
vSC (vijit.chauhan) wrote :

Hi, thanks for your inputs ..

So if I am getting clear, the BfSender_SendRaw() should call back BfSender_Connect(), in case of error.

I think BfSender_Connect() does some other things as well, apart from connecting -> what its name suggests .. It calls BfSender_SendRaw, also does the receiving of data from server ..

It could be restructured to just do just the connecting part .. a parent function could call three different functions for connecting, sending and receiving .. that way it would be easier to manage the code and make future changes easy ..

Well, I hope I have made myself clear :P

Meanwhile, I will try to change the code again as per your comment ..

> Hi and thanks for you contribution, it is really appreciated. The code is more
> or less OK, however there is one thing that doesn't behave correctly, but
> probably because there were some misunderstandings.
>
> You check the return value of BfSender_SendRaw() into BfSender_Connect().
> Since BfSender_SendRaw is used also elsewhere, I think that it would be better
> to check the return value of send() into BfSender_SendRaw() itself, and call
> _Connect() in case of errors.
>
> Feel free to ask questions if you have doubts or questions, I'm here to help.
> Also, a tip: run bzr whoami ;-)

Yes, thanks :)

>
> Thanks again for your efforts!

« Back to merge proposal