Download file and connect: if one of them fails, go to step one

My program....

• Downloads a file

• Connects to a server

If one of these operations were to fail, the program stalls for a minute and goes back to step one.

So, I would avoid recursion and the use of global variables. I think I did the first one right, but I need help to avoid the second one. What would I do if I need to "receive" variables from a function to a while loop?

```
public class Globals
{
public static int sockConn = 0;
public static bool sockRetry = false;
}

static void getInfo()
{
while (true)
{
try
{
WebRequest request = WebRequest.Create(INFO_HOST + INFO_PATH);
WebResponse response = request.GetResponse();

string content;
using (var sr = new StreamReader(response.GetResponseStream()))
{
content = sr.ReadToEnd();
}

string[] contentArray = content.Split(':');

if (contentArray.Length == 2)
{
string serverIp = contentArray[0];
string serverPortStr = contentArray[1];

int serverPort = 5000;
Int32.TryParse(serverPortStr, out serverPort);

while (Globals.sockConn == 0)
{
if (Globals.sockRetry == false)
{
connect(serverIp, serverPort);
}
else
{
Globals.sockRetry = false;
Thread.Sleep(60000);
break;
}
}

continue;
}
else
{
Thread.Sleep(60000);
continue;
}
}
catch
{
Thread.Sleep(60000);
continue;
}
}
}

static void connect(string serverIp, int serverPort)
{
Globals.sockConn = 1;

try
{
TcpClient client

Globals & Public Static Fields

public class Globals
{
    public static int sockConn = 0;
    public static bool sockRetry = false;
}

You put a beginner tag on your post, so I'm going to try do be gentle: please kill this, with fire! :)

A non-static class that's called Globals, that only contains public static fields, smells. Why? Because in an object-oriented paradigm language, globals stink and make everything more complicated than need be.

Side note: naming.

C# coding standards recommend using camelCase for local variables and parameters. Public members should be PascalCase.

OOP Basics #1: Encapsulation

Avoid exposing public fields. Always. Not because they're static - because they're fields. Fields belong to a class instance (or, when they're static, they belong to a type), and should be considered implementation details that the outside world never needs to know about. Consider systematically encapsulating fields with properties:

private int sockConn = 0;
public int SocketConnectionCount { get { return sockConn; } set { sockConn = value; } }

In this case, there isn't really a gain with the backing field - so you can have the C# compiler make one for you, leaving you with a cleaner code file:

public int SockConnectionCount { get; set; }

This is called an "auto-property". Compiler magic! :)

Thinking in Objects

Think in terms of responsibilities - what is your code accomplishing?

• Creating and sending a web request, receiving a web response.

• Streaming the response into a string.

• Splitting the string into an array; element 0 being the IP, element 1 being the port.

• Connecting to the IP/port and receiving a message through TCP/IP.

• Parsing the received message.

Break this down into their own methods/functions that do one thing, so simply that they can't possibly do it wrong. Then regroup the related ones into their own classes, and instantiate them.

Wait, something caught my eye and needs to be addressed:

int serverPort = 5000;
Int32.TryParse(serverPortStr, out serverPort);

out serverPort is an out parameter - the method it's passed into has to initialize it before it returns. In other words, the 5000 is never used. And you're never using the TryParse method's return value either (it's a Boolean that tells you whether the parse succeeded or not). out parameters are the only time you can pass an uninitialized variable into a method and have nothing to worry about. The above lines should read something like this:

int serverPort;
var result = int.TryParse(serverPortStr, out serverPort);
// do something if result is true

Avoid Hungarian-ish postfixes (and prefixes as well); If you need to specify that a variable is a String, it's because you haven't found a truly meaningful name for it. Notice I called the TryParse method on the int type. It's the very same thing as Int32 - in fact, int is a C# language shortcut for System.Int32; both are perfectly interchangeable... but it's better to choose one, and stick to it, for consistency's sake! :)

StackExchange Code Review Q#79297, answer score: 7