Soda Crate Program

Question

I would love to have some suggestion for my program, I would prefer no more suggestion to use Linq and other advanced concepts as I would like to keep it as simple as possible for now. I love suggestion on the structure of the program and methods and so on. If needed to condense the program into the basics, only the things in the menu are needed. Maybe I should use an constructor in the BeverageData class instead of property?

The Menu class is in the bottom of the code block. I've tried to split it up for readability. The code is working and without bugs as far as I know.

public enum BeverageType
    {
        Soda,
        Juice,
        Water,
        Alcohol
    }

    public class BeverageData  // Skapar 
    {
        public string Name { set; get; }
        public BeverageType Type { set; get; }
        public decimal Price { set; get; }
        public int Size { get; set; }

        public override string ToString()
        {
            return $"{Name} ({Type}) {Size}{"CL"} = {Price:C}";
        }
    }

The Beverage class, everything related to the list of beverages here.

public class Beverages
    {
        private static readonly Random Rnd = new Random();

        public static BeverageData GetBeverageFromUser()
        {
            return new BeverageData
            {
                Name = GetName("Enter name: "),
                Price = GetCurrency("Enter cost: "),
                Type = GetBeverageType("Enter type: "),
                Size = GetSize("Enter size: ")
            };
        }

        public static BeverageData GetRandomBeverage()
        {
            var names = new List<BeverageData>
        {
            new BeverageData {Name = "Coke", Price = .75m,
                Size = 12, Type = BeverageType.Soda},
            new BeverageData {Name = "Pepsi", Price = .75m,
                Size = 12, Type = BeverageType.Soda},
            new BeverageData {Name = "Sprite", Price = .75m,
                Size = 12, Type = BeverageType.Soda},
            new BeverageData {Name = "Rootbeer", Price = .75m,
                Size = 12, Type = BeverageType.Soda},
            new BeverageData {Name = "Orange Juice", Price = .5m,
                Size = 10, Type = BeverageType.Juice},
            new BeverageData {Name = "Apple Juice", Price = .5m,
                Size = 10, Type = BeverageType.Juice},
            new BeverageData {Name = "Grape Juice", Price = .5m,
                Size = 10, Type = BeverageType.Juice},
            new BeverageData {Name = "Water", Price = .25m,
                Size = 20, Type = BeverageType.Water},
            new BeverageData {Name = "Beer", Price = 2.75m,
                Size = 16, Type = BeverageType.Alcohol},
            new BeverageData {Name = "Wine", Price = 3.5m,
                Size = 9, Type = BeverageType.Alcohol},
        };

            return names[Rnd.Next(names.Count)];
        }

        private static BeverageType GetBeverageType(string message)
        {
            BeverageType beverageType;

            Console.Write(message);

            while (!Enum.TryParse(Console.ReadLine(), true, out beverageType))
            {
                Console.ForegroundColor = ConsoleColor.Red;
                Console.WriteLine("Invalid beverage type");
                Console.ResetColor();
                Console.Write("Valid beverage types are: ");
                Console.ForegroundColor = ConsoleColor.Yellow;
                Console.WriteLine(string.Join(", ", Enum.GetNames(typeof(BeverageType))));
                Console.ResetColor();
                Console.Write(message);
            }

            return beverageType;
        }

        private static string GetName(string message)
        {
            Console.Write(message);
            return Console.ReadLine();
        }

        private static decimal GetCurrency(string message)
        {
            decimal result;
            Console.Write(message);

            while (!decimal.TryParse(Console.ReadLine(), NumberStyles.Currency,
                CultureInfo.CurrentCulture, out result))
            {
                Console.ForegroundColor = ConsoleColor.Red;
                Console.WriteLine("Invalid number");
                Console.ResetColor();
                Console.Write(message);
            }

            return result;
        }
        private static int GetSize(string message)
        {
            int result;
            Console.Write(message);
            while (!int.TryParse(Console.ReadLine(), NumberStyles.Currency, CultureInfo.CurrentCulture, out result))
            {
                Console.ForegroundColor = ConsoleColor.Red;
                Console.WriteLine("Invalid Size");
                Console.ResetColor();
                Console.Write(message);
            }
            return result;
        }
    }

Here is my Beverage Crate Class, where there crate is represented and sort of the base class of the program.

class Crate : IEnumerable<BeverageData>
    {
        private BeverageData[] crate = new BeverageData[24];
        private int numberOfBottles = 0;
        private const int MaxItems = 24;

        public void Add(BeverageData beverage)
        {
            if (numberOfBottles >= MaxItems)
            {
                Console.WriteLine("The crate is full. Please remove an item first!");
            }
            else
            {
                crate[numberOfBottles] = beverage;
                numberOfBottles++;
            }
        }

        public IEnumerator<BeverageData> GetEnumerator()
        {
            return crate.AsEnumerable().GetEnumerator();
        }

        IEnumerator IEnumerable.GetEnumerator()
        {
            return GetEnumerator();
        }

        public void Remove(string name)
        {
            try
            {
                Remove(crate.FirstOrDefault(i =>
                i.Name.Equals(name, StringComparison.OrdinalIgnoreCase)));
            }
            catch (Exception)
            {
                Console.WriteLine("Please only enter name");
            }
        }

        public void Remove(BeverageData beverage)
        {
            int index = Array.IndexOf(crate, beverage, 0, numberOfBottles);
            if (index < 0)
                return;
            this.RemoveAt(index);
        }
        /// <summary>
        /// Removes the element at the specified index of the Beverage array.
        /// </summary>
        /// <param name="index">The zero-based index of the element to remove.</param>
        public void RemoveAt(int index)
        {
            if (index < numberOfBottles)
            {
                numberOfBottles--;
                Array.Copy(crate, index + 1, crate, index, numberOfBottles - index);
                crate[numberOfBottles] = default(BeverageData);
            }
        }

        public void PrintCrate()
        {
            if (numberOfBottles == 0)
            {
                Console.WriteLine("There are no items in the crate.");
            }
            else
            {
                foreach (var beverage in this)
                    Console.WriteLine(beverage);
            }
        }
        public void FindBeverageInCrate()
        {
            string crateBeverageName = Console.ReadLine();

            for (int i = 0; i < crate.Length; i++)
            {
                var bottle = crate[i];
                if (bottle == null)
                {
                    continue;
                }
                if (crate[i].Name == crateBeverageName)
                {
                    Console.WriteLine("Found your beverage {0}", crateBeverageName);
                }
            }
        }

        public void SortCrate()
        {
            int max = crate.Length;
            //Yttre loop för att få med hela 
            for (int i = 1; i < max; i++)
            {
                //Inre loop för att gå rad per rad
                int nrLeft = max - i;
                for (int j = 0; j < (max - i); j++)
                {
                    var bottle1 = crate[j];
                    var bottle2 = crate[j + 1];

                    if ((bottle1 == null) || (bottle2 == null)) // Kontrollerar att ingen av dem är tom innan den byter plats
                    {
                        continue;
                    }
                    if (bottle1.Name.CompareTo(bottle2.Name) == 1) // Byter plats 
                    {
                        var temp = crate[j];
                        crate[j] = crate[j + 1];
                        crate[j + 1] = temp;
                    }
                }
            }
        }

        public decimal CalculateTotalInCrate()
        {
            decimal summa = 0;
            foreach (var bottles in crate)
            {
                if (crate != null && bottles != null)
                    summa += bottles.Price;
            }
            Console.WriteLine("The total value of the crate is: ", summa);
            return summa;
        }

        public static int GetInt(string message)
        {
            int result;
            Console.Write(message);

            while (!int.TryParse(Console.ReadLine(), out result))
            {
                Console.ForegroundColor = ConsoleColor.Red;
                Console.WriteLine("Invalid number");
                Console.ResetColor();
                Console.Write(message);
            }

            return result;
        }
    }

And finally the menu class, the starting point of the program.*

class Menu : Crate
    {
        static void Main(string[] args)
        {
            ShowMenu();
            Console.WriteLine("\nDone!\nPress any key to exit..."); // Väntar efter input en sista gång efter användaren har tryckt 0
            Console.ReadKey();
        }
        public static void ShowMenu()
        {
            bool exit = false;
            var beverages = new Crate(); // Skapar en instans av klassen Crate som beverage
            do
            {
                Console.WriteLine("[1] Add custom beverage to array");
                Console.WriteLine("[2] Remove Bottle from Bottle Crate");
                Console.WriteLine("[3] Show Bottle Crate");
                Console.WriteLine("[4] Autofill crate");
                Console.WriteLine("[5] Sort Bottles in Bottle Crate according to name");
                Console.WriteLine("[6] Calculate the Total Cost in Crate"); 
                Console.WriteLine("[7] Search in crate after beverage");   
                Console.WriteLine("[9] Empty the crate");   
                Console.WriteLine("[0] Exit the program");


                ConsoleKeyInfo info = Console.ReadKey();
                int selection;
                int.TryParse(info.KeyChar.ToString(), out selection);

                switch (selection)  // Använder switch för att kunna se vad 
                {
                    case 1:
                        var numBevs = Math.Max(GetInt("How many beverages would you like to enter: "), 0);

                        for (int i = 0; i < numBevs; i++)
                        {
                            Console.WriteLine($"\nEnter beverage #{i + 1} info");
                            Console.WriteLine("-----------------------");
                            beverages.Add(Beverages.GetBeverageFromUser()); // Calls the Add function before the GetBeverageFromUser method
                            Console.WriteLine("-----------------------\n");
                        }
                        Console.Clear();
                        break;
                    case 2:
                        Console.Clear();
                        Console.ForegroundColor = ConsoleColor.Black;
                        Console.WriteLine("Please enter the name of the beverage you would like removed: ");
                        beverages.Remove(Console.ReadLine());
                        Console.WriteLine("Your beverage has been removed");
                        break;
                    case 3:
                        Console.Clear();
                        Console.WriteLine("\nHere are the contents of the crate:\n");
                        Console.ForegroundColor = ConsoleColor.Green;
                        beverages.PrintCrate();
                        Console.ResetColor();
                        break;

                    case 5:
                        numBevs = Math.Max(GetInt("How many would you like to be auto-added: "), 0);

                        for (int i = 0; i < numBevs; i++)
                        {
                            beverages.Add(Beverages.GetRandomBeverage());
                        }
                        Console.WriteLine("-----------------------\n");
                        Console.Clear();
                        break;
                    case 6:
                        Console.Clear();
                        beverages.SortCrate();
                        Console.WriteLine("The Crate has been sorted");
                        break;
                    case 7:
                        Console.ForegroundColor = ConsoleColor.Red;
                        beverages.CalculateTotalInCrate();
                        Console.ResetColor();
                        break;
                    case 8:
                        beverages.FindBeverageInCrate();
                        Console.WriteLine("This is your beverage?");
                        break;
                    case 9:
                        ShowMenu();
                        Console.Clear();
                        Console.WriteLine("Your crate has been emptied");
                        break;
                    case 0:
                        exit = true;
                        break;
                    default:
                        Console.Clear();
                        Console.WriteLine("An error has occured, try again");
                        break;
                }
            } while (!exit);
        }
    }

Answer 1

Class name BeverageData feels clumsy to me. I would simply go with Beverage. It's Size could be renamed to include its units to add clarity. Since Size is an int, your model breaks apart when you encounter a beverage that has non-integer size.

This line of code:

return $"{Name} ({Type}) {Size}{"CL"} = {Price:C}";

should be:

return $"{Name} ({Type}) {Size}CL = {Price:C}";

The biggest change I would recommend to you is for you to focus on Separation of Concerns and the Single Responsibility Principle. So many of your methods begin with an action word like "GetSomething", and while they do get that something, they also write to the console. These Get methods should just GET. Leave it to another line of code to worry about console writes.

You should switch from arrays to lists. Your Crate class could simply use the underlying list.RemoveAt instead of you transforming your own array. Likewise, use the list.Sort method to reduce your code making it more readable to others.

You may want to change your FindBeverage method to allow the comparison to be case-insensitive.

EDIT TO QUESTIONS IN COMMENT:

Beverage and Beverages are distinguishable by one being singular and the other being plural or a collective.

An example of outputting a string rather the write to the console would be:

Old:

public void PrintCrate()
{
    if (numberOfBottles == 0)
    {
        Console.WriteLine("There are no items in the crate.");
    }
    else
    {
        foreach (var beverage in this)
            Console.WriteLine(beverage);
    }
}

New:

public string DisplayCrate()
{
    if (numberOfBottles == 0)
    {
        return "There are no items in the crate.";
    }
    else
    {
        var sb = new StringBuilder();
        foreach (var beverage in this)
            sb.AppendLine(beverage);
        return sb.ToString();
    }
}

Why return a string rather than jump dump to console? What if 6 months from now you want to convert your console app to a WinForm or WPF or web app? Your Crate class shouldn't be concerned with how something is presented. It should only concern itself with what it to be presented.

Switching from arrays to lists:

Old:

private BeverageData[] crate = new BeverageData[24];
private int numberOfBottles = 0;
private const int MaxItems = 24;

public void Add(BeverageData beverage)
{
    if (numberOfBottles >= MaxItems)
    {
       Console.WriteLine("The crate is full. Please remove an item first!");
    }
    else
    {
       crate[numberOfBottles] = beverage;
       numberOfBottles++;
    }
}

New:

    private const int MaxItems = 24;
    private List<BeverageData> crate = new List<BeverageData>(MaxItems);

    public void Add(BeverageData beverage)
    {
        if (create.Count >= MaxItems)
        {
            throw new Exception("The crate is full. Please remove an item first!");
        }
        else
        {
            crate.Add(beverage);
        }
    }

You no longer need to track numberOfBottles. Your SortCrate can be shortened to:

   public void SortCrate()
   {
       crate = crate.OrderBy(x => x.Name).ToList();
   }

And you may even add your own custom comparer (left as homework). Other things left as homework: keep in mind you find beverages by name. It is possible to have beverages with the same name but with different sizes.

That's all the time I have for now.

EDIT 2:

Sorry I am doing this in pieces, but real life takes priority. I was hoping others would have chimed in by now.

The Crate class could implement List<BeverageData> rather than IEnumerable<BeverageData>. Again code would be simplified and easier to follow.

Harping again on Separation of Concerns, things like the FindBeverageInCrate method should not be concerned with prompting for a beverage name. There should be no ReadLine in that method. You should just pass a string beverageName.

Also that method would be better named as FindBeverageByName. The InCrate is not needed since you are in the Crate class. And the name clearly says what its doing: finding a beverage by name. Your homework is to consider what you would do if you allowed 2 beverage instances of the same name?

You Menu class should not implement Crate. Again, Separation of Concerns are at play here. You should instead have a Crate instance defined within the class.

There is a lot more that can be mentioned, but it gets too long and is overkill. Try a reworking with some of these notes and post a "Ver 2" in a few days. I'd be glad to review that one too.

EDIT 3

Here is a quick re-working of the a few of the classes.

// General categories of Soft versus Hard (Alcohol). 
// Why not: Beer, Wine, Liquor?
public enum BeverageType
{
    Soda,
    Juice,
    Water,
    Alcohol
}

public class Beverage: IComparable<Beverage>, IEquatable<Beverage>
{
    public string Name { set; get; }
    // Type is poor property name since it means something else in .NET
    public BeverageType Category { set; get; }
    public decimal Price { set; get; }
    public Single Liters { get; set; }

    // Implement IComparable.CompareTo
    public int CompareTo(Beverage other)
    {
        if (other == null)
        {
            return 1;
        }
        int compare = string.Compare(Name, other.Name, true);
        if (compare != 0)
        {
            return compare;
        }
        compare = Category.CompareTo(other.Category);
        if (compare != 0)
        {
            return compare;
        }
        compare = Liters.CompareTo(other.Liters);
        if (compare != 0)
        {
            return compare;
        }
        return Price.CompareTo(other.Price);
    }

    // Implement IEquatable.Equals
    public bool Equals(Beverage other)
    {
        return CompareTo(other) == 0;
    }

    public override string ToString()
    {
        return $"{Name} ({Category}) {Liters:N2} Liters = {Price:C}";
    }
}

public class Crate : List<Beverage>
{
    private const int DefaultBottleLimit = 24;
    public int BottleLimit { get; }
    public int Id { get; }

    public Crate(int id) : this(id, DefaultBottleLimit) { }
    public Crate(int id, int limit)
    {
        // should have checks that limit > 0
        Id = id;
        BottleLimit = limit;
    }

    public new void Add(Beverage beverage)
    {
        if (Count >= BottleLimit)
        {
            throw new Exception("The crate is full. Please remove an item first!");
        }
        base.Add(beverage);
    }

    public int NumberOfBottles => Count;
    public int EmptySlots => BottleLimit - Count;

    public void Remove(string name)
    {
        var candidate = FindBeverageByName(name);
        if (candidate == null)
        {
            // Do you silently return or throw exception?
        }

        base.Remove(candidate);
    }

    public override string ToString()
    {
        var sb = new StringBuilder();
        sb.AppendLine($"Crate ID: {Id}, Total Slots: {BottleLimit}, Bottles: {NumberOfBottles}");
        foreach (var beverage in this)
        {
            sb.AppendLine($"\t{beverage}");
        }
        return sb.ToString();
    }

    public Beverage FindBeverageByName(string name)
    {
        foreach (var beverage in this)
        {
            if (string.Compare(beverage.Name, name, true) == 0)
            {
                return beverage;
            }
        }
        return null;
    }

    public decimal CalculateTotal()
    {
        return this.Sum(x => x.Price);
    }
}

Of particular mention:

The BeverageType enum could break alcohol into 3 more types: Beer, Wine, and Liquor.

The Beverage class now implements IComparable and IEquatable. This CompareTo becomes the default comparer later when working with List<Beverage>.

The Type property was renamed to Category to avoid confusion with System.Type.

Size was renamed to Liters.

The Crate class had dramatic changes, and is much shorter in length. It now implements List<Beverage>. Because you want a limit on the number of bottles in a crate, we had to override the Add method accordingly. Note that it also calls the base.Add method.

I added an Id property and a BottleLimit property. I made these read only properties. In the future you may deal with many crates, so an Id helps distinguish them.

You will note there is no Remove(Beverage beverage) method. Well, there is. It's inherited from List<Beverage> and required no special coding.

I did have to modify Remove<string name>. Note how simplified it is thanks to the modified FindBeverageByName which returns a Beverage instance or a null. Note this returns the first found, if any.

CaculateTotal is simpler thanks to LINQ.

The PrintCrate method is now ToString but includes the basic crate info before showing the inventory.

What about Crate.Sort? Thanks to Beverage.CompareTo, you may sort a crate instance such as:

var crate = new Crate(1);
// add bottles to crate using crate.Add
crate.Sort();

That's all you have to do. Let the framework do the heavy lifting for you. Your code will be shorter and easier to understand. Mind you, what I have here is far from perfect. But it is a good starting platform. Read it. Understand it. And then see how you would integrate this alongside a menu system.