I feel dirty...

Harvey Saayman

... I used my first goto today :shudders: to workaround this problem[^] I've been having.

    private void BroadcastPacket(string Packet, FSClient client)
    {
        List ClientsToRemove = new List();

        ASCIIEncoding encoder = new ASCIIEncoding();
        byte\[\] PacketBytes = encoder.GetBytes(Packet);

        lock (ClientLock)
        {
            foreach (FSClient CurrentClient in clients)
            {
                if (client != CurrentClient)
                {
                    try
                    {
                        if (CurrentClient.Socket.Connected)
                        {
                        TryAgain:
                            try
                            {
                                CurrentClient.Socket.Send(PacketBytes, PacketBytes.Length, SocketFlags.None);
                                bytesSent += PacketBytes.Length;
                            }
                            catch (SocketException ex)
                            {
                                if (ex.SocketErrorCode == SocketError.WouldBlock)
                                {
                                    Console.WriteLine("SocketError.WouldBlock Occured");
                                    Thread.Sleep(300);
                                    goto TryAgain;
                                }
                            }
                        }
                        else
                        {
                            ClientsToRemove.Add(CurrentClient);
                        }
                    }
                    catch (Exception ex)
                    {
                        Console.WriteLine("Error @ BroadcastPacket(): " + ex.Message);
                        ClientsToRemove.Add(CurrentClient);
                    }
                }
            }
        }

        foreach (FSClient CurrentClient in ClientsToRemove)
        {
            CloseConnection(CurrentClient);
        }
    }

I'm going straight to hell :((

Roger Wright

There is nothing inherently wrong with the goto statement. I've had a number of clients in the past that I wish now I had used that statement for, as part of a slightly larger function.

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

leppie

You should feel dirty, but not for the goto. That is perfect for these. You should feel like a sewer rat for that infinite indentation levels you just exposed our eyes to! (maybe the tab size is a notable problem too, time to move to 2 spaces) Loops and that are easy to refactor.

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

Rajesh R Subramanian

The indentation levels... Ugh. X| You could use an infinite loop and a break condition within the loop instead, but that's a goto in disguise anyways. :)

“Follow your bliss.” – Joseph Campbell

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