Calculate the area of different shapes using polymorphism (virtual and override methods)

Question

I have written code to calculate the area of different shapes using polymorphism (virtual and override methods) and it is working as expected. Can anyone please review this and let me know how I can make it better?

class Program
{
    static void Main(string[] args)
    {
        double side = 0;
        double length = 0;
        double width = 0;
        double height = 0;
        double baseoftriangle = 0;
        double radius = 0;

        UserChoice:
        Console.WriteLine("For what shape you want to calculate the Area:\n1. Sqaure\n2. Rectangle\n3. Triangle\n4. Circle");
        Console.Write("Please Select the number from above options: ");
        int choice = int.Parse(Console.ReadLine());



        switch (choice)
        {
            case 1:
                Console.Write("Please enter the side of square: ");
                side = double.Parse(Console.ReadLine());
                break;
            case 2:
                Console.Write("Please enter the length of rectangle: ");
                length = double.Parse(Console.ReadLine());
                Console.Write("Please enter the width of rectangle: ");
                width = double.Parse(Console.ReadLine());
                break;
            case 3:
                Console.Write("Please enter the height of triangle: ");
                height = double.Parse(Console.ReadLine());
                Console.Write("Please enter the base of triangle: ");
                baseoftriangle = double.Parse(Console.ReadLine());
                break;
            case 4:
                Console.Write("Please enter the radius of circle: ");
                radius = double.Parse(Console.ReadLine());
                break;
            default:
                Console.WriteLine("Incorrect Choice, please try again!");
                goto UserChoice;
        }

        CalculateArea Sqa = new Square();
        CalculateArea Rec = new Rectangle();
        CalculateArea Tri = new Triangle();
        CalculateArea Cir = new Circle();
        if (choice == 1)
        {
            Sqa.Area(side);
            Sqa.ShowResult();
        }
        else if(choice==2)
        {
            Rec.Area(length,width);
            Rec.ShowResult();
        }
        else if(choice==3)
        {
            Tri.Area(height, baseoftriangle);
            Tri.ShowResult();
        }
        else
        {
            Cir.Area(radius);
            Cir.ShowResult();
        }


        ChoiceOfAnotherCalculation:
        Console.Write("\nDo you want to calculate area of any other shape? Give input in Yes or NO: ");
        string choice1 = Console.ReadLine();

        switch (choice1.ToUpper())
        {
            case "YES":
                goto UserChoice;
            case "NO":
                break;
            default:
                Console.WriteLine("Incorrect Choice, please try again!");
                goto ChoiceOfAnotherCalculation;
        }
    }
}
class CalculateArea
{
    public double result;
    public virtual void Area(double side)
    {
    }
    public virtual void Area(double length, double width)
    {
    }
    public void ShowResult()
    {
        Console.WriteLine($"Your Result is {result}");
    }
}
class Square: CalculateArea
{

    public override void Area(double side)
    {
        result = side * side;
    }

}
class Rectangle:CalculateArea
{
    public override void Area(double length, double width)
    {
        result = length * width;
    }
}
class Triangle:CalculateArea
{
    public override void Area(double height, double baseoftriangle)
    {
        result = (height * baseoftriangle)/2;
    }
}
class Circle:CalculateArea
{
    public override void Area(double radius)
    {
        result = 3.14159 * radius * radius;
    }
}

Answer 1

As it stands there is unfortunately a lot of code-smell in here, first lets take a look at your inheritance:

class CalculateArea
{
    public double result;
    public virtual void Area(double side)
    {
    }
    public virtual void Area(double length, double width)
    {
    }
    public void ShowResult()
    {
        Console.WriteLine($"Your Result is {result}");
    }
}

This class should be marked abstract since there is no reason that anybody would ever want to create a CalculateArea object. You should also not rely on your implementors to use public variables. Area should return the area, not set the result field.

The other problem with the base class is the use of two different methods. Computing the area of a circle doesn't make sense to have a length and width overload, but you are dictating that your classes have that. You should reduce your interface to the lowest common denominator. Let the implementation's worry about what kind of area to compute, just dictate that it has an Area property.

That would turn your CalculateArea class into something like this:

public abstract class HasAreaBase
{
    public abstract int Area { get; }
}

Also notice I changed the name. Also notice how I left out the ShowResult method. This isn't a good way to do that, what if you wanted different text? Or only the number? In a different language? Try not to be too specific in implementation, leave that up to the "end" user.

Actually instead of an abstract class, I think this would be better off as an interface, this is what it would look like:

public interface IHasArea
{
    int Area { get; }
}

But since we are learning about abstraction/inheritance I'll leave it as a class.

Now lets tackle the implementations, I'll do two of them here, you can figure out the rest:

public class Rectangle : HasAreaBase
{
    public override int Area { get { return Length * Width; } }
    public int Length { get; set; }
}

public class Triangle : HasAreaBase
{
    public override int Area { get { return (Height * BaseWidth) / 2; } }
    public int Height { get; set; }
    public int BaseWidth { get; set; }
}

So now we can look at the main method of your class. It is doing way too much. Let's break that down a little bit. Your program needs to do 3 things:

• Ask the user for a choice
• Construct the shape based on that choice
• Display the result to the user
• Repeat ask for a choice

So we'll make the program look like this:

public static class Program
{
    public enum MenuChoice
    {
        Quit = 0,
        Rectangle = 1,
        Triangle = 2
    }

    public static void Main(string[] args)
    {
        MenuChoice choice = MenuChoice.Triangle;
        while ((choice = GetChoice()) != MenuChoice.Quit)
        {
            ComputesAreaBase shape = null;

            switch (choice)
            {
                case MenuChoice.Triangle:
                    shape = CreateTriangle();
                    break;
                case MenuChoice.Rectangle:
                    shape = CreateRectangle();
                    break;
                default:
                    break;
            }

            Console.WriteLine("The area of the {0} is {1}", choice, shape.Area);
            Console.WriteLine();
        }

        Console.ReadKey(true);    
    }

    public static MenuChoice GetChoice()
    {
        Console.WriteLine("Choose which shape you would like to calculate the area for:");
        Console.WriteLine("\t{0} - Rectangle", (int)MenuChoice.Rectangle);
        Console.WriteLine("\t{0} - Triangle", (int)MenuChoice.Triangle);
        Console.WriteLine("\t{0} - Quit", (int)MenuChoice.Quit);
        Console.Write("Choice: ");
        string value = Console.ReadLine();

        MenuChoice choice;
        if (!Enum.TryParse(value, out choice) || !Enum.IsDefined(typeof(MenuChoice), choice))
        {
            Console.WriteLine("Invalid choice, please enter a valid choice.");
            return GetChoice();
        }

        return choice; 
    }

    private static int GetInt(string prompt)
    {
        bool isValid = false;
        int value = 0;

        while (!isValid)
        {
            Console.Write(prompt);
            isValid = int.TryParse(Console.ReadLine(), out value);
        }

        return value;
    }

    private static Triangle CreateTriangle()
    {
        int baseWidth = GetInt("Base Width: ");
        int height = GetInt("Height: ");
        return new Triangle() { BaseWidth = baseWidth, Height = height };  
    }

    private static Rectangle CreateRectangle()
    {
        int width = GetInt("Width: ");
        int height = GetInt("Height: ");
        return new Rectangle() { Width = width, Height = height };    
    }
}

So, going through this one-by-one:

• GetInt will get any integer from the prompt. This reduces the amount of code you need to write to get an integer. In your implementation you've allowed the user to enter any value and assume it is an integer (try entering "fun" once, the program will break).
• GetChoice now prints out the menu and gets the choice. It uses an enum for the choices which is easily extensible. You could abstract this even more and automatically print the menu, but I think you get the point.
• CreateRectangle and CreateTriangle are their own methods, which they should be. These return the concrete class back to Main, but through the beauty of inheritance, stores it in the base ComputesAreaBase object.
• Main is now wonderfully short and understandable. It asks the user for a choice (via GetChoice) until the user chooses quit. Based on the choice it will get the correct shape. The rest of the method doesn't know what shape was created, but it is able to use the shape anyway to display the area.

This shows the power of inheritance a little better. MSDN also has a good example using shapes as well that you may want to read.

The take-away here are these points:

• If you are using goto, then you are almost always using it wrong. There are very few legitimate uses for goto, and if that comes into mind, please try to think of a better method for control flow.
• If you are using inheritance but creating derived classes, you are probably using it wrong. Again you want to use the class highest up in the inheritance chain that contains the functionality that you need.
• In your example everything is based on int, but for some reason you are double.TryParse-ing, which is losing precision. Either use all integers, or all doubles, but don't mix-and-match.
• Don't assume that the user is going to enter a number when you want a number, or a string when you want a string. Always validate it and display an error. int.Parse will throw an exception on invalid input, int.TryParse will not.
• Do not change the names of variables when you inherit them. This, in my opinion, is a poor design choice by the C# team to allow that. The problem is that when referring to it by the abstract base class, the original names appear, and will not have meaning. Doesn't make sense for a Triangle to have a length and width when using it as the base class right?