Do you guys think this code will function properly?

public void LoadUser(string UserName, string Password)
        {
            try
            {
                using (StreamReader sr = new StreamReader("./Users/" + UserName + ".txt"))
                {
                    string Line;
                    bool passCorrect = false;
                    bool userCorrect = false;

                    while ((Line = sr.ReadLine()) != null)
                    {
                        if (Line.Equals("UserName = " + UserName))
                        {
                            userCorrect = true;
                        }
                        if (Line.Equals("Password = " + Password))
                        {
                            passCorrect = true;
                        }
                        if (userCorrect && passCorrect)
                        {
                            MessageBox.Show("Username and password are correct");
                        }
                    }
                }
            }
            catch (Exception)
            {
                MessageBox.Show("Account doesn't exist.", "Login", MessageBoxButtons.OK, MessageBoxIcon.Error);
            }
        }
private void button1_Click(object sender, EventArgs e)
        {
            string user = textBox1.Text;
            string pass = textBox2.Text;
            LoadUser(user, pass);
        }

And this is an example of the textfile:

UserName = Jayden
Password = lolcats

Thanks in advance!

Edited 6 Years Ago by Jaydenn: n/a

Not sure why you would ask this question rather than just test out your code to see if it does indeed work, but the code is nicely structured for the most part.

  • Your test for valid username/password will display a message box for every line read following the initial successful read of those two line items (username/password), but if those are the only two lines I suppose that wouldn't matter... You might consider having the method return a bool (success or not) that would allow the calling code to display message box and determine whether the user can continue in the program...
  • You might consider using a single configuration file for all users instead of a separate file for each... Also, naming the file same as username, then reading the line to compare the username is redundant.
  • Not sure you can use forward slash (eg. '/'), or single dot then slash (eg. "./" for relative) for the path portion of the string, but maybe it works?
  • You really should use encryption or another way to store/obscure the password (at a minimum) for obvious reasons.

If you are interested in improving the security and design, there are many articles and posts available here and elsewhere on the web.

Edited 6 Years Ago by DdoubleD: n/a

In addition to what DdoubleD said why are you catching all exceptions and treating that as "account does not exist"? Is that to handle an IOException if the file doesn't exist? If so then you should test to see if the file exists before attempting to read. Throwing exceptions is a very expensive process and should be avoided wherever possible.

Also take a look at System.IO.Path.Combine() for joining path strings instead of concatening a string manually.

This question has already been answered. Start a new discussion instead.