3
\$\begingroup\$

I'm very new to programming and C# and had some homework this weekend to create a simple string check. I've expanded upon that and came up with this. This is way more complex than what was asked but I wanted to push myself and see what I can learn.

I would like feedback and criticism for what I am doing right and wrong. This script is a basic login and register an account in C# console app and it saves a text file to remember users.

using System;
using System.Collections;
using System.Linq;
using System.Text;
using System.IO;

namespace additionalProgramming2_
{
    class Program
    {
        static void Main(string[] args)
        {
            string input;
            string savePath = (@"F:\nick Programing\additionalProgramming2 +\Save\");
            int ID = 0;
            bool login = false;

            string[] usernameArray = File.ReadAllLines(@"F:\nick Programing\additionalProgramming2+\Save\username.txt");//loads a text file and sets it to an array
            ArrayList username = new ArrayList(usernameArray);//sets the array to an array list
            string[] passwordArray = File.ReadAllLines(@"F:\nick Programing\additionalProgramming2+\Save\password.txt");
            ArrayList password = new ArrayList(passwordArray);
            string[] timeArray = File.ReadAllLines(@"F:\nick Programing\additionalProgramming2+\Save\time.txt");
            ArrayList time = new ArrayList(timeArray);

        start:
            if (login == true)
            {
                goto menu;
            }
            Console.Clear();
            Console.WriteLine(@"What would you like to do?
1 Login
2 Register
3 Shut Down");
            input = Console.ReadLine();

            switch(input)
                {
                case "1":
                case "login":
                    Console.WriteLine("What is your user name?");
                    input = Console.ReadLine();
                    input = input.ToLower();
                    if (input =="default")
                    {
                        Console.WriteLine("Please try another user name");
                        Console.ReadKey();
                        goto start;
                    }
                    foreach (string name in username)//runs through the username list
                    {
                        if (name == input)//returns true if it finds a match in the list
                        {
                            int listNo = username.IndexOf(input);//sets the listNo to the index number of the password list that matched
                            Console.WriteLine("What is your password?");
                            input = Console.ReadLine();
                            string passCheck = Convert.ToString(password[listNo]);//sets the passCheck var to the string index no found at the same index as the user name
                            if (input == passCheck) //if the input and the passCheck are the same you logged in
                                {
                                ID = listNo;//sets the ID for the user
                                string lastLogin = Convert.ToString(time[ID]);//gets the last login from the time list
                                        Console.WriteLine(@"You logged in!
You last logged in at "+lastLogin );
                                time[ID] = (Convert.ToString(DateTime.Now));//sets a new login time
                                using (TextWriter writer = File.CreateText(@"F:\nick Programing\additionalProgramming2+\Save\time.txt"))//creates a txt file called time
                                  {
                                  foreach (string date in time)
                                    {
                                    writer.WriteLine(date);//adds a new line to the txt file for time
                                    }
                                  }
                                   Console.ReadKey();
                                login = true;
                                   goto start;
                                }
                        }
                    }
                   Console.WriteLine("Sorry there was some error!");
                    Console.ReadKey();
                    goto start;

                case "2":
                case "register":

                    Console.WriteLine("What would you like your user name to be?");
                    username:
                    input = Console.ReadLine();
                    input = input.ToLower();
                    if (input == "")
                    {
                        Console.WriteLine("Please input a username");
                        goto username;
                    }
                    foreach(string name in username)
                    {
                        if(name == input)//checks if there is a user name called that already
                        {
                            Console.WriteLine("Sorry this username is taken");
                            Console.ReadKey();
                            goto start;
                        }
                    }
                    username.Add(input);//adds the username to the username list
                    Console.WriteLine("What would you like your password to be?");
                    password:
                    input = Console.ReadLine();
                    if (input == "")
                    {
                        Console.WriteLine("Please enter a password");
                        goto password;
                    }
                    password.Add(input);//adds the password to the password list
                    using (TextWriter writer = File.CreateText(@"F:\nick Programing\additionalProgramming2+\Save\username.txt"))//creates a txt file called username
                    {
                        foreach (string name in username)
                        {
                            writer.WriteLine(name);//adds a new line to the txt file for the user
                        }
                    }
                    using (TextWriter writer = File.CreateText(@"F:\nick Programing\additionalProgramming2+\Save\password.txt"))
                    {
                        foreach (string pass in password)
                        {
                            writer.WriteLine(pass);
                        }
                    }
                    time.Add(Convert.ToString(DateTime.Now));
                    using (TextWriter writer = File.CreateText(@"F:\nick Programing\additionalProgramming2+\Save\time.txt"))//creates a txt file called username
                    {
                        foreach (string date in time)
                        {
                            writer.WriteLine(date);//adds a new line to the txt file for the user
                        }
                    }
                    Console.WriteLine("User created!");
                    Console.ReadKey();
                    break;

                case "3":
                case "shutdown":
                    Console.Clear();
                    Console.WriteLine("Shutting down");
                    Console.ReadKey();
                    Environment.Exit(0);//closes down the console
                    break;

                default:
                    Console.WriteLine("Unexpected input");
                    Console.ReadKey();
                    break;
            }
            goto start;

        menu:
            Console.Clear();

            string user = Convert.ToString(username[ID]);
            Console.WriteLine(@"Main menu 
Welcome back " + user);
            Console.WriteLine(@"
1 logout
2 ChangePassword
3 Shutdown");
            input = Console.ReadLine();
            input.ToLower();
            switch(input)
            {
                case "1":
                case "logout":
                    Console.WriteLine("Would you like to logout? y/n");
                    input = Console.ReadLine();
                    if (input == "y")
                    {
                        login = false;
                        ID = 0;
                        Console.WriteLine("Logged out");
                        Console.ReadKey();
                    }
                    break;

                case "2":
                case "changepassword":
                    Console.WriteLine("What would you like your new password to be?");
                    input = Console.ReadLine();
                    password[ID] = input;
                    using (TextWriter writer = File.CreateText(@"F:\nick Programing\additionalProgramming2+\Save\password.txt"))
                    {
                        foreach (string pass in password)
                        {
                            writer.WriteLine(pass);
                        }
                    }
                    Console.WriteLine("Password changed!");
                    Console.ReadKey();
                    break;

                case "3":
                case "shutdown":
                    Console.Clear();
                    Console.WriteLine("Shutting down");
                    Console.ReadKey();
                    Environment.Exit(0);//closes down the console
                    break;

                default:
                    Console.WriteLine("Unexpected input");
                    Console.ReadKey();
                    break;
            }
            goto start;
        }
    }
}
\$\endgroup\$
3
\$\begingroup\$

Welcome to Code Review!

God function
You have all code in one function, the main. That's what sometimes is called a God function: It knows and does too much. Try splitting your code up into smaller, isolated pieces with clear tasks. If you look at the flow of your code, I'm sure you can find several of these tasks:

  • Read values from a file into an array or list,
  • Print the menu
  • Register a user
  • Login a user

Don't use goto
Goto is mostly considered bad form. It's very easy to make so called spaghetti code, which is very hard to maintain. There are a heap of other flow control statements to use instead: for and while loops, switch statements, whatnot. For a CLI menu like you have, I'd say that the most common is to use a while loop:

var quit = false;
while (!quit)
{
    var input = readInput()
    if ("exit".eaquls(input))
    {
        quit = true;
    }
}

Generics are your friend
You use the namespace System.Collections for the ArrayList class. I'd use List<T> from System.Collections.Generic instead. It will let you specify the type of objects ending up in your list. User names for example are strings, but with ArrayList you can add numbers, other array lists, whatever. See e.g. here for more on generics. You probably don't want this:

    ArrayList users = new ArrayList();
    users.Add(1);
    users.Add(new ArrayList());
    users.Add("test");

This is better:

var better = new List<string>();
better.Add("Alice");
better.Add("Bob");

Now the compiler will warn you ahead of time, before you try using an ArrayList as a user name.

\$\endgroup\$
  • \$\begingroup\$ Thank you for taking the time to read though my code and give feedback. I will try and apply what you have said this week to new scripts I try and make. It makes sense what you have said about the god code, i hadn't really known how to apply proper methods, i'v looked it up and i think i kinda understand it, i'v tried applying it to this code but I just mess everything up too much, Going forward ill try it out! \$\endgroup\$ – nozpher Oct 21 at 7:38

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.