HiveBrain v1.2.0
Get Started
← Back to all entries
debugcsharpMinor

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

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
placesamountdecimalinputroundedgetthatanddisplayinteger

Problem

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;
        }
    }
}

Solution

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.");
        }
    }
}

Code Snippets

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();
    }
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.");
        }
    }
}

Context

StackExchange Code Review Q#94875, answer score: 5

Revisions (0)

No revisions yet.