I feel dirty...

[Mark_Wallace]

Well, it was either that or use a wait until statement -- but that would have been trickier, because they don't exist. Anyway, why should you feel dirty for using a statement correctly and wisely? Do you feel dirty for the if statement you used? Don't let the knee-jerks of idiots catch you in the palloncini.

I wanna be a eunuchs developer! Pass me a bread knife!

[Harvey Saayman]

I'm going to change the way that works now... The server has a main server thread that kicks off the implicit threads(BeginXXX & EndXXX). Then in the ReceiveData callback i just shove the raw data into a collection that sits in the client object and gets processed by another thread that manages a threadpool for processing(and at the moment I also execute BroadcastPacket() in there) I'm thinking of having another thread pool that just sits and sends messages cause if some of the clients are a bit slow, it holds up all the others. The goto is fine? :doh:

Harvey Saayman - South Africa Software Developer .Net, C#, SQL you.suck = (you.Occupation == jobTitles.Programmer && you.Passion != Programming) 1000100 1101111 1100101 1110011 100000 1110100 1101000 1101001 1110011 100000 1101101 1100101 1100001 1101110 100000 1101001 1101101 100000 1100001 100000 1100111 1100101 1100101 1101011 111111

[Roger Wright]

Harvey Saayman wrote:

The goto is fine?

Yes. It's there for a reason, and if there was no good reason for using it, the language probably wouldn't support it. GOTO got its well-deserved bad reputation from the terrible spaghetti code written in BASIC, but even modern languages and programming styles have an occasional need for an absolutely deterministic JMP equivalent. You've implemented it in an appropriate context, so carry on.

"A Journey of a Thousand Rest Stops Begins with a Single Movement"

[leppie]

Harvey Saayman wrote:

The goto is fine?

Of course :) There is a place and time for everything. Some people might say, you could refactor the goto out. Personally I prefer readability.

xacc.ide
IronScheme - 1.0 RC 1 - out now!
((λ (x) `(,x ',x)) '(λ (x) `(,x ',x))) The Scheme Programming Language – Fourth Edition

[leppie]

Rajesh R Subramanian wrote:

The indentation levels...

:)

xacc.ide
IronScheme - 1.0 RC 1 - out now!
((λ (x) `(,x ',x)) '(λ (x) `(,x ',x))) The Scheme Programming Language – Fourth Edition

[CPallini]

goto? No problem! On the other hand, those annoying try-catch blocks around the brave goto statement... :rolleyes: Don't be afraid of using gotos. Klingon developers use goto-hell here and there (in fact Klingons use C, the only language of real men...). Strange enough, after 40 (forty!) years since the (in)famous criticisms to the goto statement, the MS 'fireblade' language still provides it... :laugh:

If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler. -- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong. -- Iain Clarke
[My articles]

[Christian Graus]

I'd use a while loop to write that code. And I'd break it into functions, so that it's not so nested.

Christian Graus Driven to the arms of OSX by Vista. Read my blog to find out how I've worked around bugs in Microsoft tools and frameworks.

[Rajesh R Subramanian]

CPallini wrote:

Klingon developers use goto-hell

:laugh:

“Follow your bliss.” – Joseph Campbell

[Pete OHanlon]

I'd definitely consider refactoring the code if I were you, but not because of the goto statement. The Thread.Sleep is a bit of a no-no though - have a read of this thread[^] (pardon the pun).

"WPF has many lovers. It's a veritable porn star!" - Josh Smith

As Braveheart once said, "You can take our freedom but you'll never take our Hobnobs!" - Martin Hughes.

My blog | My articles | MoXAML PowerToys | Onyx

[Abhinav S]

MS suggests using goto to get out of deeply nested loops [^] :^) . So I suppose using it here should have no issues.

There's nothing left in my right brain and nothing right in my left brain.

[Spoon Of Doom]

CPallini wrote:

in fact Klingons use C, the only language of real men...

Mind if I quote that and use it everywhere I go? :laugh:

[Dave Parker]

You could use a boolean flag and a while loop. But I don't see anything wrong with goto really and goto is probably how I'd do it. I sometimes wonder if there's anything bad about using them in catch blocks though, but haven't ever come across or read about any problems associated with it.

[Simon P Stevens]

Interesting. I've never come across that before. The one place I have used Thread.Sleep is where I have a .net app that is communicating with a horrible nasty old server from circa 1825 via telnet. The nasty old server in question runs an app that is so rubbish it needs messages to arrive slowly so it has time to process them. Hence my implementation of communicating with it is pretty much this:

Connection con = OpenConnection("StupidServer");
SendMessage("Hi");
Thread.Sleep(1000);
SendMessage("Do something");
Thread.Sleep(1000);
SendMessage("Do yet another pointless and stupid thing");
Thread.Sleep(1000);
SendMessage("I don't want to talk to you anymore, you are dull and boring. Goodbye");
Thread.Sleep(1000);
CloseConnection(con);

(server name and messages changes to protect identity) I don't really care about exact timings, I just need to make sure my messages don't go too fast or the server will blow up. Rewriting the server to add in ready acknowledgements, or the ability to queue messages or something like that is not an option. All this does happen on a background thread so the GUI isn't being blocked. I think that is a pretty valid use of Thread.Sleep. Anyone disagree? Is there an obvious better way I've missed.

Simon

[Pete OHanlon]

Here's a none-Goto version:

private static readonly object _syncLock = new object();

private void BroadcastPacket(string Packet, FSClient client)
{
List<FSClient> ClientsToRemove = new List<FSClient>();
byte[] PacketBytes = new ASCIIEncoding().GetBytes(Packet);
bool retry;

lock (ClientLock)
{
foreach (FSClient currentClient in clients)
{
try
{
if (client != currentClient)
{
do
{
retry = ProcessPacket(PacketBytes);
} while (retry);
}
else
{
ClientsToRemove.Add(client);
}
}
catch
{
ClientsToRemove.Add(client);
}
}
foreach (FSClient CurrentClient in ClientsToRemove)
{
CloseConnection(CurrentClient);
}
}
}

private bool ProcessPacket(byte[] packetBytes)
{
bool retry = false;
try
{
CurrentClient.Socket.Send(packetBytes, packetBytes.Length, SocketFlags.None);
bytesSent += packetBytes.Length;
}
catch (SocketException ex)
{
if (ex.SocketErrorCode == SocketError.WouldBlock)
{
retry = true;
lock (_syncLock)
{
Monitor.Wait(_syncLock, 300);
}
}
}
return retry;
}

I typed this in Notepad, so it might have a couple of errors, but it should do the same thing as the goto version.

"WPF has many lovers. It's a veritable porn star!" - Josh Smith

As Braveheart once said, "You can take our freedom but you'll never take our Hobnobs!" - Martin Hughes.

My blog | My articles | MoXAML PowerToys | Onyx

[N a v a n e e t h]

It looks OK in this case. The problem with Thread.Sleep is that it is not guaranteed to sleep for the specified time. It may take few ticks more than what specified. But since you don't care about exact timings, it should be a valid usage. One alternative method is to use WaitHandle and pass a timeout value to WaitOne method.

Best wishes, Navaneeth

[Pete OHanlon]

If I were coding this thing from scratch I'd use something like a Monitor, and a background thread to handle this. I'd use Monitor.Wait to wait for the child thread to poke the parent to wake it up - something like this:

lock (_syncLock)
{
SendMessage("Hi"); // SendMessage operates on a background thread, and uses Pulse(_syncLock) to
// notify the parent thread that it's finished processing.
Monitor.Wait(_syncLock, 1000);
}

Basically, by doing this you can wake the parent up if you finish processing earlier than 1000 milliseconds, or the parent drops out at roughly 1 second. You could always use an Infinite timeout here to wait until the child thread has completed processing.

"WPF has many lovers. It's a veritable porn star!" - Josh Smith

As Braveheart once said, "You can take our freedom but you'll never take our Hobnobs!" - Martin Hughes.

My blog | My articles | MoXAML PowerToys | Onyx

[leppie]

For your next exercise, convert the above to use CPS, and use success, retry and failure continuations. Bonus points if you can implemented it in C# :)

xacc.ide
IronScheme - 1.0 RC 1 - out now!
((λ (x) `(,x ',x)) '(λ (x) `(,x ',x))) The Scheme Programming Language – Fourth Edition

[Simon P Stevens]

Pete O'Hanlon wrote:

by doing this you can wake the parent up if you finish processing earlier than 1000 milliseconds

That's the problem. SendMessage will almost certainly take less that 1000 ms, but if I send the next message straight away the server will get overloaded. The server can only handle one thing at a time, it accepts a message (and acknowledges it's been received). Then goes off and does whatever you told it to do, which can take a while, and then it becomes ready to receive the next message. If you send a second message while it's still handling the first it blows up.

Simon

[Rajesh R Subramanian]

The deep nesting is unnecessary as far as this particular piece of code is concerned. I'm sure it could be refactored. For instance, a simple while loop with a break condition must reduce a bit of the clutter and remove the goto also.

“Follow your bliss.” – Joseph Campbell

[Simon P Stevens]

N a v a n e e t h wrote:

use WaitHandle and pass a timeout value to WaitOne method.

Yeah, I suppose that's a way of avoiding Thread.Sleep, but it's essentially the same thing. I'd just be blocking the thread at the WaitOne until the timeout as I would never be signalling the wait handle because the server never sends any kind of message to let me know it's ready. I just have to pause for a bit and hope it's ready if I give it long enough.

Simon