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

Question

(I've improved the code from my last question)

My program....

1. Downloads a file (web tracker)
2. Update if version is lower
3. Connects to a server using the tracker information

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

A few wrong things I can notice for myself:

1. responseStr: this doesn't look a good variable name, any ideas?
2. I've to replace Thread.Sleep, use Timer instead.

public const double APP_VERSION = 0.76;

public const string INFO_HOST = "http://*********.appspot.com";
public const string INFO_PATH = "/tracker.txt";

[STAThread]
static void Main()
{
    Thread connection = new Thread(WebFile.Download);
    connection.Start();

    // ...
}

public class WebFile
{
    public static void Download()
    {
        System.Globalization.CultureInfo customCulture = (System.Globalization.CultureInfo)System.Threading.Thread.CurrentThread.CurrentCulture.Clone();
        customCulture.NumberFormat.NumberDecimalSeparator = ".";

        System.Threading.Thread.CurrentThread.CurrentCulture = customCulture;

        while (true)
        {
            try
            {
                WebRequest request = WebRequest.Create(INFO_HOST + INFO_PATH + "?v=" + APP_VERSION);
                WebResponse response = request.GetResponse();

                string responseStr = streamResponse(response);
                parseResponse(responseStr);
            }
            catch
            {
                Thread.Sleep(60000);
                continue;
            }
        }
    }

    private static string streamResponse(WebResponse response)
    {
        string content;
        using (var streamReader = new StreamReader(response.GetResponseStream()))
        {
            content = streamReader.ReadToEnd();
        }

        return content;
    }

    private static void parseResponse(string response)
    {
        if (response.Length > 1 && response.Substring(0, 1) != "#")
        {
            Thread.Sleep(60000);
            return;
        }

        string[] contentArray = response.Substring(1).Split(',');

        if (contentArray.Length >= 2)
        {
            if (contentArray.Length == 4)
            {
                String updateVersionStr = contentArray[2];
                String downloadURL = contentArray[3];

                double updateVersion;
                var updateVersionResult = double.TryParse(updateVersionStr, out updateVersion);

                if (updateVersionResult && updateVersion > APP_VERSION)
                {
                    String dataFolder = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData),
                                            "Client");

                    using (WebClient client = new WebClient())
                    {
                        try
                        {
                            client.DownloadFile(downloadURL, dataFolder + "/update.exe");
                            Process.Start(dataFolder + "/update.exe");
                            Environment.Exit(0);
                        }
                        catch { }
                    }
                }
            }

            string serverIp = contentArray[0];
            string serverPortStr = contentArray[1];

            int serverPort;
            var serverPortResult = int.TryParse(serverPortStr, out serverPort);

            if (serverPortResult == false)
            {
                serverPort = 5000;
            }

            ConnectionInfo connectionInfo = new ConnectionInfo();

            while (connectionInfo.SockConnectionCount == 0)
            {
                if (connectionInfo.SockConnectionRetry == false)
                {
                    Connection.Connect(serverIp, serverPort, connectionInfo);
                }
                else
                {
                    connectionInfo.SockConnectionRetry = false;
                    break;
                }
            }

            Thread.Sleep(60000);
            return;
        }
        else
        {
            Thread.Sleep(60000);
            return;
        }
    }
}

public class ConnectionInfo
{
    public int SockConnectionCount { get; set; }
    public bool SockConnectionRetry { get; set; }
}

public class Connection
{
    public static void Connect(string serverIp, int serverPort, ConnectionInfo connectionInfo)
    {
        connectionInfo.SockConnectionCount = 1;

        try
        {
            TcpClient client = new TcpClient(serverIp, serverPort);
            NetworkStream nwStream = client.GetStream();
            StreamReader reader = new StreamReader(nwStream);
            StreamWriter writer = new StreamWriter(nwStream);

            connectionInfo.SockConnectionCount = 2;

            writer.WriteLine("V:" + APP_VERSION);
            writer.Flush();

            writer.WriteLine("C:cmd");
            writer.Flush();

            writer.WriteLine("U:" + Environment.UserName + "," + System.Environment.MachineName);
            writer.Flush();

            string inputLine;

            while ((inputLine = reader.ReadLine()) != null)
            {
                parseMessage(writer, serverIp, inputLine);
            }
        }
        catch
        {
            connectionInfo.SockConnectionRetry = true;
            connectionInfo.SockConnectionCount = 0;
            return;
        }
    }

    private static void parseMessage(StreamWriter Writer, string serverIp, string msg)
    {
        // ...
    }
}

Answer 1

Some quick remarks:

• APP_VERSION, INFO_HOST, INFO_PATH: constants should use PascalCase.
• streamResponse, parseResponse, parseMessage: method names should use PascalCase
• streamResponse also doesn't really do that; I'd go with ReadResponse.
• downloadURL: capitalize only the first character of acronyms with three or more characters
• "/update.exe" is repeated, should be a constant so you'll only need to update it in one place if you ever need to.
• Folder name "Client" should be a constant.
• Why assign serverIp and serverPortStr when you use them only once?
• SockConnectionCount? SockConnectionRetry? Why not use the full "Socket"?
• Wouldn't it be better to encapsulate NetworkStream, StreamReader and StreamWriter in a using statement? (But be careful not to close the stream.)

---

Reduce indentation in methods if possible: apply the same as you did for if (response.Length > 1 && response.Substring(0, 1) != "#") to if (contentArray.Length >= 2):

string[] contentArray = response.Substring(1).Split(',');

if (contentArray.Length < 2)
{
    Thread.Sleep(60000);
    return;
}

if (contentArray.Length == 4)
{
   // etc.

That way a maintainer doesn't have to scroll 60+ lines just to find out that the else to that if is merely yet another sleep + return.

---

Why assign serverPortResult when you only use it once? Instead of:

int serverPort;
var serverPortResult = int.TryParse(serverPortStr, out serverPort);
if (serverPortResult == false)
{
    serverPort = 5000;
}

... do this:

int serverPort;
if (!int.TryParse(serverPortStr, out serverPort))
{
    serverPort = 5000;
}

---

Again you use two lines when one would be enough:

TcpClient client = new TcpClient(serverIp, serverPort);
NetworkStream nwStream = client.GetStream();

Could be just this:

NetworkStream nwStream = new TcpClient(serverIp, serverPort).GetStream();

And why not call it networkStream? No need for abbreviations.

Answer 2

Just noticed this and noticed the one thing the other answer omitted was:

System.Globalization.CultureInfo customCulture = (System.Globalization.CultureInfo)System.Threading.Thread.CurrentThread.CurrentCulture.Clone();

It is acceptable to initialize variables using var keyword to avoid long redundant declarations. And I don't believe using the Clone() method is the best way to get a copy of the current culture. Try this instead:

var customCulture = new CultureInfo(CurrentCulture.Name);

You should be able to avoid the namespace identifiers (e.g. System.Globalization) if you put them at the top of the file in the using declarations.