Basic ATM Software from a Udemy Project

Question

Recently starting to learn programming, I'm looking to move to making larger projects and am just picking up the basics now. I have a feeling this code sort of missed the point? Am looking for feedback for better/ more efficient practices to make it more functional/clean.

The following was the prompt:

1. Simple ATM Software This simple project will essentially create a simulation of an ATM within a Windows program. Just like an ATM, the program should have at least the following features:

• Checking whether an input – such as an ATM card (a debit/credit card number) – is recorded correctly
• Verifying the user by asking for a PIN
• In case of negative verification, logging out the user
• In case of positive verification, showing multiple options, including cash availability, the previous five transactions, and cash withdrawal
• Giving the user the ability to withdraw up to $1,000 worth of cash in one transaction, with total transactions limited to ten per day. (I omitted the total of ten transactions)

    using System;
    using System.Collections.Generic;
    using System.Linq;
    using System.Text;
    using System.Threading.Tasks;
    using System.Threading;

    namespace ATM_SOFTWARE
    {
        internal class Program
        {

            static void Main(string[] args)
            {

            //Check for a valid ATM input

            int cardNumber;
            Boolean validCard = false;


            while (validCard == false)
            {
                Console.WriteLine("");
                Console.WriteLine("!!INPUT YOUR SIX DIGIT ACCOUNT NUMBER!!");
                string digits = Console.ReadLine();
                cardNumber = digits.Length;

                if (cardNumber == 6)
                {
                    Console.WriteLine("");
                    Console.WriteLine("!!VALID ACCOUNT NUMBER!!");
                    validCard = true;

                }
                else
                {
                    Console.WriteLine("");
                    Console.WriteLine("!!INVALID ACCOUNT NUMBER. PLEASE TRY AGAIN!!");
                    validCard = false;

                }
            }

            //Verify the user by asking for a pin
            int PIN;
            Boolean validPin = false;

            while (validPin == false)
            {
                Console.WriteLine("");
                Console.WriteLine("PLEASE ENTER YOUR 4 DIGIT PIN.");
                string passCode = Console.ReadLine();
                PIN = passCode.Length;

                if (PIN == 4)
                {
                    //Result from Positive (Transaction Options)
                    Console.WriteLine("");
                    Console.WriteLine("!!VALID PIN NUMBER. WELCOME USER!!");
                    validPin = true;

                }
                else
                {
                    //or Negative (Log them out) verification
                    Console.WriteLine("");
                    Console.WriteLine("!!INVALID PIN NUMBER. MALICIOUS ACTIVITY DETECTED. TERMINATING USER!!");
                    validPin = false;
                    for (int i = 0; i < 5; i++)
                    {
                        Console.WriteLine("");
                        Console.WriteLine("!!DEATH! DEATH! DEATH!!");

                    }
                    Console.ReadKey();
                    System.Environment.Exit(1);
                    break;
                }
            }

            //Withdrawal options, up to $1000 in one transaction, ten per day
            Boolean status = true;

            int balance = 1000;

            while (status == true)
            {
                int selection;

                Console.WriteLine("");
                Console.WriteLine("-----------------------");
                Console.WriteLine("");
                Console.WriteLine("!!WELCOME THEN, MEAT. EXHANGE YOUR FRIVOLOUS CURRENCY. CHOOSE YOUR OPTION!!");
                Console.WriteLine("!!CURRENT BALANCE " + (balance) + " EURODOLLARS!!");
                Console.WriteLine("");
                Console.WriteLine("-----------------------");
                Console.WriteLine("");
                Console.WriteLine("!!OPTION 1: WITHDRAW!!");
                Console.WriteLine("!!OPTION 2: DEPOSIT!!");
                Console.WriteLine("!!OPTION 3: TERMINATE APPLICATION!!");

                selection = Convert.ToInt32(Console.ReadLine());

                switch (selection)
                {
                    case 1:
                        Console.WriteLine("");
                        Console.WriteLine("-----------------------");
                        Console.WriteLine("");
                        Console.WriteLine("!!CURRENT BALANCE " + (balance) + " EURODOLLARS!!");
                        Console.WriteLine("");

                        Console.WriteLine("!!HOW MUCH DO YOU WANT TO WITHDRAW!!");

                        int withdraw;

                        withdraw = Convert.ToInt32(Console.ReadLine());

                        if (withdraw <= balance)
                        {
                            balance -= withdraw;
                            Console.WriteLine("");
                            Console.WriteLine("!!DISPENSING CURRENCY!!");
                            Console.WriteLine("");
                            Console.WriteLine("-----------------------");
                            Console.WriteLine("");
                            Console.WriteLine("!!CURRENT BALANCE " + (balance) + " EURODOLLARS!!");
                            Console.WriteLine("!!PRESS ANY KEY TO RETURN!!");
                            Console.ReadKey();
                            break;
                        }

                        else
                        {
                            Console.WriteLine("");
                            Console.WriteLine("!!NICE TRY MEAT, YOU DON'T HAVE THAT MUCH!!");
                            Console.WriteLine("!!PRESS ANY KEY TO RETURN!!");
                            Console.ReadKey();
                            break;
                        }

                    case 2:
                        Console.WriteLine("");
                        Console.WriteLine("-----------------------");
                        Console.WriteLine("");
                        Console.WriteLine("!!CURRENT BALANCE " + (balance) + " EURODOLLARS!!");
                        Console.WriteLine("");

                        Console.WriteLine("!!HOW MUCH DO YOU WANT TO DEPOSIT!!");

                        int deposit;

                        deposit = Convert.ToInt32(Console.ReadLine());

                        balance += deposit;

                        Console.WriteLine("");
                        Console.WriteLine("!!CURRENT BALANCE " + (balance) + " EURODOLLARS!!");
                        Console.WriteLine("!!PRESS ANY KEY TO RETURN!!");
                        Console.ReadKey();
                        break;

                    case 3:
                        Console.WriteLine("");
                        Console.WriteLine("!!THANK YOU. PRESS ANY KEY TO CLOSE THE APPLICATION!!");
                        Console.ReadKey();
                        status = false;
                        break;
                        
                    default:
                        Console.WriteLine("");
                        Console.WriteLine("!!INVALID SELECTION, TRY AGAIN!!");
                        Console.ReadKey();
                        break;

                }


            }
        }
    }
}

Answer 1

As mentioned in Peter Csala's comment, first thing I'd do is to split out the Main components.

Doing this first pretty clearly highlights some flow issues (IMO) with your code:

internal class Program
{
    static void Main(string[] args)
    {
        ValidateCard();
        ValidatePIN();
        ProcessTransactions();
    }
    // Implement methods
}

So, right off, is this the way you want the program to flow? The only way to exit the program is to enter a valid card AND a valid PIN and only then do you get the option to quit?

internal class Program
{
    static void Main(string[] args)
    {
        if( ValidateCard() && ValidatePIN() )
        {
            ProcessTransactions();
        }
    }
}

Already this seems more straightforward; if you fail to ValidateCard(), then the program exits because the second conditional isn't evaluated for && operators if the first conditional evaluates to false. If the card is valid but the ValidatePIN() fails, again you terminate the program.

You can still loop inside those conditionals, but now you can easily introduce the exit mechanism inside each step. You can loop until true is returned if you don't want the user to abort the program, but now you can optionally add the capability to return false (like if you fail to validate a PIN three times, etc.) and abort the program.

I'm assuming the code all works generally as you like, so the only other comment I've got is that your strings are clogging up your code. I would use the line feed character \n instead of using a Console.WriteLine("");, just because of the space it takes up, and then also that lets you define prompts up front, so you could convert the ProcessTransaction() code to something like this:

void PrintBalance(int balance)
{
    Console.WriteLine("!!CURRENT BALANCE " + (balance) + " EURODOLLARS!!")
}

void ProcessTransactions()
{
    int balance = 1000;

    string bar = "\n-----------------------\n";
    string pressAnyKey = "!!PRESS ANY KEY TO RETURN!!";
    string invalidSelection = "\n!!INVALID SELECTION, TRY AGAIN!!";
    string transactionOptions = "!!OPTION 1: WITHDRAW!!\n" + 
                                "!!OPTION 2: DEPOSIT!!\n" + 
                                "!!OPTION 3: TERMINATE APPLICATION!!";

    string greeting = "!!WELCOME THEN, MEAT. EXHANGE YOUR FRIVOLOUS CURRENCY. CHOOSE YOUR OPTION!!";
    string goodbye = "\n!!THANK YOU. PRESS ANY KEY TO CLOSE THE APPLICATION!!";

    string withdrawPrompt = "!!HOW MUCH DO YOU WANT TO WITHDRAW!!";
    string withdrawSuccess = "\n!!DISPENSING CURRENCY!!";
    string withdrawFailure = "\n!!NICE TRY MEAT, YOU DON'T HAVE THAT MUCH!!";

    string depositPrompt = "\n!!HOW MUCH DO YOU WANT TO DEPOSIT!!";

    //Withdrawal options, up to $1000 in one transaction, ten per day
    Boolean status = true;

    
    while (status == true)
    {
        int selection;

        Console.WriteLine(bar);
        Console.WriteLine(greeting);
        PrintBalance(balance);
        Console.WriteLine(bar);
        Console.WriteLine(transactionOptions);

        selection = Convert.ToInt32(Console.ReadLine());

        switch (selection)
        {
            case 1:
                Console.WriteLine(bar);
                PrintBalance(balance);
                Console.WriteLine(withdrawPrompt);

                int withdraw;

                withdraw = Convert.ToInt32(Console.ReadLine());

                if (withdraw <= balance)
                {
                    balance -= withdraw;
                    Console.WriteLine(withdrawSuccess);
                    Console.WriteLine(bar);
                    PrintBalance(balance);
                    Console.WriteLine(pressAnyKey);
                    Console.ReadKey();
                    break;
                }

                else
                {
                    Console.WriteLine(withdrawFailure);
                    Console.WriteLine(pressAnyKey);
                    Console.ReadKey();
                    break;
                }

            case 2:
                Console.WriteLine(bar);
                PrintBalance(balance);

                Console.WriteLine(depositPrompt);

                int deposit;

                deposit = Convert.ToInt32(Console.ReadLine());

                balance += deposit;

                PrintBalance(balance);
                Console.WriteLine(pressAnyKey);
                Console.ReadKey();
                break;

            case 3:
                Console.WriteLine(goodbye);
                Console.ReadKey();
                status = false;
                break;
                
            default:
                Console.WriteLine(invalidSelection);
                Console.ReadKey();
                break;

        }
    }
}

Now, does this make the code shorter? Not really! What it does, though, is make the code more readable, which counts for a lot. Now you can look at where the strings are defined and look at your patterns. For example, you have the "unpaired" prompts:

string bar = "\n-----------------------\n";
string pressAnyKey = "!!PRESS ANY KEY TO RETURN!!";
string invalidSelection = "\n!!INVALID SELECTION, TRY AGAIN!!";
string transactionOptions = "!!OPTION 1: WITHDRAW!!\n" + 
                            "!!OPTION 2: DEPOSIT!!\n" + 
                            "!!OPTION 3: TERMINATE APPLICATION!!";

Then you have "paired" prompts, like greeting/goodbye:

string greeting = "!!WELCOME THEN, MEAT. EXHANGE YOUR FRIVOLOUS CURRENCY. CHOOSE YOUR OPTION!!";
string goodbye = "\n!!THANK YOU. PRESS ANY KEY TO CLOSE THE APPLICATION!!";

And you have the withdrawal prompt/success/failure:

string withdrawPrompt = "!!HOW MUCH DO YOU WANT TO WITHDRAW!!";
string withdrawSuccess = "\n!!DISPENSING CURRENCY!!";
string withdrawFailure = "\n!!NICE TRY MEAT, YOU DON'T HAVE THAT MUCH!!";

And you have the deposit prompt/success/failure:

string depositPrompt = "\n!!HOW MUCH DO YOU WANT TO DEPOSIT!!";

But wait! There's no success/failure for deposits like there are for withdrawals. In looking at the rewritten code:

case 2:
    Console.WriteLine(bar);
    PrintBalance(balance);

    Console.WriteLine(depositPrompt);

    int deposit;

    deposit = Convert.ToInt32(Console.ReadLine());

    balance += deposit;

    PrintBalance(balance);
    Console.WriteLine(pressAnyKey);
    Console.ReadKey();
    break;

What happens if Convert.ToInt32 fails there? This may have been previously overlooked because the text is !!SHOUTING AND DISTRACTING!!. By moving the text out, the structure becomes more apparent, and again you find bugs by making your code more readable.

Personally not a fan of the SHOUTING PROMPTS! and if it were me I'd probably wrap everything, like:

void Shout(string text)
{
    Console.WriteLine(text.ToUpper());
}

and then you can have the text formatted easier to read and can shout it when it's displayed, but that's borderline personal preference. I think, again, making the text more readable will make it more likely for you to read it, which in turn makes it more likely to find typos, but I get you're going for a style so that's up to you.

Actually, in reviewing what I wrote, I noticed there's also the issue of

selection = Convert.ToInt32(Console.ReadLine());

Here again, like in processing the deposit, you're blindly parsing with no TryParse and no try...catch, so if the user didn't enter something that can be cast to Int32 then your program crashes. I'd look at Int32.TryParse(Console.ReadLine()) and you can handle failures if TryParse returns false.

Then, in looking at the other Convert.ToInt32, your deposit is being read as Int32 instead of Uint32, meaning the user could add negative money. Not a problem to deposit negative money, but they could withdraw negative money. Negative one million is less than their balance, and "withdrawing" negative one million adds a million when you do balance -= withdraw, and then the user could withdraw positive one million. To my point again, though (I'll keep harping on it!) it's easier to focus on the code when you aren't distracted by reading the flavor text in the prompts.