Quick lookover

[frontlinegamerz]

I'm not an expert with c# but i am on my own learning. Can anyone look at the following code and tell me how it looks?

// START ----- CHECK FOR FILES AND LOAD
try
{
    if (File.Exists("data.dat"))
        using (StreamReader sr = new StreamReader("data.dat"))
        {
            string line;
            while ((line = sr.ReadLine()) != null)
            {
                // insert data into system
            }

        }
    else
    {
        Mbox msgbox = new Mbox();
        msgbox.Text = "First Time Setup";
        msgbox.Show();
        // create file
        using (FileStream fs = File.Create("data.dat"))
        {
            msgbox.Close();
        }

    }
}
catch (Exception)
{

    throw;
}

 

[Skydiver]

There is no point in catching an exception just to throw it again. You can get rid of the try-catch.

I assume that MBox is your own custom message box. Why not use the default MessageBox class? Or is it just something like a progress bar dialog box? Do you even really need it? In general the Windows guidelines are that you just change the cursor to a wait cursor if something will use up some time, but the time is more than 2-3 seconds. If something will take more than 10 seconds, then along with the wait cursor, some kind progress indicator is recommended.

Comments should explain something that is not obvious. Commenting the obvious just add visual clutter and does not actually help make the code more understandable. If your comment applies to a big block of code like you have above, the modern convention is to move that code into a method and make the method name say what is happening. In your case, something like LoadOrCreateGameData().

Move your filename into a constant string variable and use that variable instead. That way if you ever change your filename, there is only one place to change it instead of trying to hunt around for everywhere the name might be.

You need to be consistent about your use of curly braces. If you have curly braces around the body of the else body because it is multiple statements, you should also have curly braces around the if body even if semantically everything with the using is just a single statement.

In general keep functions less than 20-25 lines long. If the get longer, break up the parts into multiple methods. Like in the case above, the if body could move into another method for loading, and the else body can move into another method for creating.

 

[Skydiver]

Your lines 5-13 could probably be replaced with:

foreach(var line in File.ReadLines(GameFileName))
{
    // insert data into system
}