Get an integer as input and display Pi rounded to that amount of decimal places

Question

I've been writing a program that accepts an integer as input and displays the number Pi rounded to that number. The only issue I see is the fact that the Math.Round method can only round up to 15 spaces and there's no try-catch for that ArgumentOutOfRange exception. I'm also not sure how safe it is letting your flow of execution rely on a try-catch statement.

class Program
{
 public static int counter = 0;
 static void Main(string[] args)
 {
 Console.WriteLine("Welcome to the Pi-Rounder! Find Pi to up to 15 decimal places!");
 Console.WriteLine("Please enter the number of decimal places you'd like to round to.");
 int roundTo;
 do
 {
 string digitAsString = Console.ReadLine();
 roundTo = ConversionLoop(digitAsString);
 }
 while(roundTo == 0 && counter != 5);
 if(counter == 5)
 {
 throw new FormatException();
 }
 else
 {
 double piRounded = Math.Round(Math.PI, roundTo);
 Console.WriteLine(piRounded);
 Console.ReadLine();
 }
 }
 static int ConversionLoop(string digitString)
 {
 try
 {
 int digit = Convert.ToInt32(digitString);
 return digit;
 }
 catch(FormatException)
 {
 counter++;
 Console.WriteLine("That was not a valid number. Please try again.");
 return 0;
 }
 }
}

• dfhwze: Srinivasa Aaiyangar Ramanujan made such a function, using nothing but math to estimate PI to n digits. youtube.com/watch?v=IjmRfalFU70

Answer 1

There are several issues with this piece of code:

 do
 {
 string digitAsString = Console.ReadLine();
 roundTo = ConversionLoop(digitAsString);
 }
 while(roundTo == 0 && counter != 5);
 if(counter == 5)
 {
 throw new FormatException();
 }
 else
 {
 double piRounded = Math.Round(Math.PI, roundTo);
 Console.WriteLine(piRounded);
 Console.ReadLine();
 }

Problems:

• "ConversionLoop" is a meaningless name. The function parses a string to an integer, so a better name would be toInt
• The handling of invalid input and incrementing the counter are not visible here. At first look I didn't see how the counter can advance, and it seemed you don't tell the user about invalid results. I had to look at the ConversionLoop to find out, but it was not logical to do so. The responsibility of getting valid input should not be split between two methods, it would be clearer to handle in one place, and have all the elements of the logic easily visible.
• If the user fails to enter valid input 5 times, the code throws new FormatException()
  • FormatException is not appropriate for this. The problem is not invalid format, but failure to enter valid input within a reasonable number of retries. It's a different kind of error, and should be captured by a different exception class
  • Creating an exception without a text message explaining the problem makes debugging difficult
• After throwing an exception in the if branch, the program exits from the method, so you can simplify the else

A bug

I think you have a bug: if the user enters 0 as input, the ConversionLoop method doesn't print an error and returns 0 normally, but the program will still wait for another try. Without a message, this will be confusing to the user. I doubt you intended it this way.

Suggested implementation

With the above suggestions, the code becomes:

class UserInputException : Exception
{
 public UserInputException(string message) : base(message)
 {
 }
}
public static int MAX_TRIES = 5;
static void Main(string[] args)
{
 Console.WriteLine("Welcome to the Pi-Rounder! Find Pi to up to 15 decimal places!");
 int roundTo = ReadIntegerInput();
 double piRounded = Math.Round(Math.PI, roundTo);
 Console.WriteLine(piRounded);
 Console.ReadLine();
}
static int ReadIntegerInput() 
{
 Console.WriteLine("Please enter the number of decimal places you'd like to round to.");
 int counter = 0;
 while (true)
 {
 string digitAsString = Console.ReadLine();
 try
 {
 return Convert.ToInt32(digitAsString);
 }
 catch(FormatException)
 {
 if (++counter == MAX_TRIES)
 {
 throw new UserInputException("Too many invalid inputs. Goodbye.");
 }
 Console.WriteLine("That was not a valid number. Please try again.");
 }
 }
}

• Risky Martin: One other problem is the code never checks if the input integer is too high or too low.
• janos: Good point. It would be a legit answer, I think

Answer 2

if(counter == 5)
{
 throw new FormatException();
}

I'm not sure why you throw an unhandled exception in Main to make your program crash here when an error message and return would suffice. On the other hand, why are you limiting the number of invalid input the user can enter at all? If I understand this correctly, they get 5 invalid inputs total, but otherwise they can input as many values as they want? Why make a limit at all?

---

This is an unnecessary try/catch:

try
{
 int digit = Convert.ToInt32(digitString);
 return digit;
}

You should use the int.TryParse(string, out int) method. This will assign the value to the out int if possible (an out argument must be assigned in the method), and return a Boolean value for success:

int digit;
if (int.TryParse(digitString, out digit)
{
 return digit;
}
else
{
 counter++;
 Console.WriteLine("That was not a valid number. Please try again.");
 return 0;
}

---

I don't like that output in ConversionLoop(). If you have to display that error somewhere, throw an exception and let the caller display the error, or a custom error, as they see fit.