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

Revision history for this message
Andrea Corbellini (andrea.corbellini) wrote :

On Thu, Jul 15, 2010 at 7:25 PM, vSC <email address hidden> wrote:
> So if I am getting clear, the BfSender_SendRaw() should call back BfSender_Connect(), in case of error.

Yeah, right. Probably it'd be useful some code to avoid recursive
loops, but this is something we can think about later.

> 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 ..

It's true that BfSender_Connect() doesn't just connect: in fact it
connects, then sends a request to server and finally checks the
response. And this should be done every time. If the connection is
lost, you should both connect and send the request. For this reason, I
don't think that I'd be useful to split BfSender_Connect() into three
functions would be useful: connecting without sending the request
wouldn't produce the expected result.

Do you agree? Feel free to say you don't, of course ;-)

> Well, I hope I have made myself clear :P
>
> Meanwhile, I will try to change the code again as per your comment ..

Sure, everything's clear. Thanks!

--
Ubuntu member  | http://www.ubuntu.com/
BeeSeek member | http://www.beeseek.org/

« Back to merge proposal