Reputation: 27
I'm having an issue with calling methods from another class in C#. I haven't been doing this long (3 weeks to be exact) and don't get the ideas behind calling methods. I have declared everything as public to try to make it easier but it isn't working for me just yet. Any help at all would be very much appreciated. Here is the code in question, I want to use a method in an if statement to calculate the area of various simple shapes however at the output stage I get "That is not a valid choice"
namespace Area_Calculator
{
public class Area
{
public static int Square(int side)
{
int i, A;
Console.WriteLine("Please enter the length of the side of the square");
i = Convert.ToInt16(Console.ReadLine());
A = i * i;
return A;
}
public static int Rectangle(int width, int height)
{
int i, j, A;
Console.WriteLine("Please enter the width of the rectangle");
i = Convert.ToInt16(Console.ReadLine());
Console.WriteLine("Please enter the height of the rectangle");
j = Convert.ToInt16(Console.ReadLine());
A = i * j;
return A;
}
public static double Triangle(int width, int height)
{
double i, j, A;
Console.WriteLine("Please enter the width of the triangle");
i = Convert.ToDouble(Console.ReadLine());
Console.WriteLine("Please enter the height of the triangle");
j = Convert.ToDouble(Console.ReadLine());
A = (.5 * i * j);
return A;
}
public static double Circle(int radius)
{
int i;
double A;
Console.WriteLine("Please enter the radius of the circle");
i = Convert.ToInt16(Console.ReadLine());
A = (i * Math.PI);
return A;
}
}
class Program
{
static void Main(string[] args)
{
int x, i, j;
i = 0;
j = 0;
Console.WriteLine("Please select what type of shape you wish to find the area of:\n1. Square\n2. Rectangle\n3. Triangle\n4. Circle\n");
x = Convert.ToInt16(Console.ReadLine());
Area r = new Area();
if (x == 1)
{
Area.Square(i);
}
if (x == 2)
{
Area.Rectangle(j, i);
}
if (x == 3)
{
Area.Triangle(j, i);
}
if (x == 4)
{
Area.Circle(i);
}
else
{
Console.WriteLine("That is an invalid choice");
}
Console.ReadKey();
}
}
}
Upvotes: 2
Views: 157
Reputation: 56
enum AreaEnum
{
Square,
Rectangle,
Triangle,
Circle,
};
class Program
{
static void Main(string[] args)
{
Console.WriteLine("Please select what type of shape you wish to find the area of:\n1. Square\n2. Rectangle\n3. Triangle\n4. Circle\n");
int x = Convert.ToInt16(Console.ReadLine());
AreaEnum myValueAsEnum = (AreaEnum)x;
Calculate(myValueAsEnum);
}
static double Calculate(AreaEnum a)
{
int x, i, j;
i = 0;
j = 0;
Area area = new Area();
switch (a)
{
case AreaEnum.Square:
{
return Area.Square(i);
}
case AreaEnum.Rectangle:
{
return Area.Rectangle(j, i);
}
case AreaEnum.Triangle:
{
return Area.Triangle(j, i);
}
case AreaEnum.Circle:
{
return Area.Circle(i);
}
default:
{
Console.WriteLine("That is an invalid choice");
return 0;
}
}
}
}
Upvotes: 0
Reputation: 11273
The problem is with your if
statement, you need to be using else if
instead of if
:
if (x == 1)
{
Area.Square(i);
}
else if (x==2)
{
Area.Rectangle(j, i);
}
else if (x == 3)
{
Area.Triangle(j, i);
}
else if (x==4)
{
Area.Circle(i);
}
else
{
Console.WriteLine("That is an invalid choice");
}
The issue when using if is that it got to the very last if and the else
was true, so it would print "That is an invalid choice".
And some notes on your implementation...
In your main program, its unnecessary to have an Area
object, Area r = new Area()
, since you don't use r
anywhere.
All of the methods in Area
take in values (which you pass in) but then completely ignore them and ask for the values again. I would either remove the parameters from the methods, or ask the user in the main program and pass in the values. The input logic can be put inside the if
statement for each different calculation. Last, you don't do anything with the return value of the function, so you don't display the area to the user. You need to edit your functions to write it to the console, for example:
Console.WriteLine("The square area is {0}", Area.Square());
Inside the if statement, or because you are doing user input in the calculation, you could have a similar line in each Area
method.
Upvotes: 0
Reputation: 4928
Your code is checking if x equals 4, otherwise using the code in the else block.
This will be run every time unless x=4!
Try this instead:
x = Convert.ToInt16(Console.ReadLine());
Area r = new Area();
if (x == 1)
{
Area.Square(i);
}
else if (x==2)
{
Area.Rectangle(j, i);
}
else if (x == 3)
{
Area.Triangle(j, i);
}
else if (x==4)
{
Area.Circle(i);
}
else
{
Console.WriteLine("That is an invalid choice");
}
or even better:
x = Console.ReadLine();
switch x
{
case "1":
Area.Square(i);
break;
case "2":
Area.Rectangle(j, i);
break;
case "3":
Area.Triangle(j, i);
break;
case "4":
Area.Circle(i);
break;
default:
console.WriteLine("That is an invalid choice");
break;
}
Upvotes: 0
Reputation: 3088
Your main issue is, what others have mentioned, about the if statements. Another thing is that you calculate the area but never print it out.
if (x == 1)
{
Console.WriteLine(Area.Square(i));
}
else if (x == 2)
{
Console.WriteLine(Area.Rectangle(j, i));
}
else if (x == 3)
{
Console.WriteLine(Area.Triangle(j, i));
}
else if (x == 4)
{
Console.WriteLine(Area.Circle(i));
}
else
{
Console.WriteLine("That is an invalid choice");
}
Upvotes: 2
Reputation: 53958
Initially, you have to change a bit your if statements, like below:
if (x == 1)
{
Area.Square(i);
}
else if (x == 2)
{
Area.Rectangle(j, i);
}
else if (x == 3)
{
Area.Triangle(j, i);
}
else if (x == 4)
{
Area.Circle(i);
}
else
{
Console.WriteLine("That is an invalid choice");
}
Doing so, you will get the message that you get know in all cases, where x
is not 4, also in the rest cases 1, 2 and 3.
Upvotes: 0
Reputation: 1503529
You'll currently always see "That is an invalid choice" unless x is 4... because the final if
/`else is disconnected from all the rest.
You could change it to use else if
everywhere, like this:
if (x == 1)
{
...
}
else if (x == 2)
{
...
}
else if (x == 3)
{
...
}
else if (x == 4)
{
...
}
else
{
...
}
... but it would be simpler to use a switch
statement:
switch (x)
{
case 1:
...
break;
case 2:
...
break;
case 3:
...
break;
case 4:
...
break;
default:
...
break;
}
That better expresses your intention of "I want to execute exactly one of these branches, based on a simple selection on x
, with a "default" branch if x
isn't any of the known values."
Upvotes: 7