Converting the weight of a potato into a letter grade

Question

I've just started learning C# using Rob Miles' C# Programming Yellow Book and some related lab exercises from his website. I did one of them and produced a solution that works. In Miles' book, he says that nesting if and else brackets is clumsy and introduces the switch. But switch apparently doesn't work with Boolean operators, e.g. case x < 201, so I can't use it. I could just have independent, non-nested if statements, though their conditions would have to be longer. Any other ideas?

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;


class Program
{
    static int GetIntNumber(int min, int max, string prompt, string inputPrompt)
    {
        int number;

        Console.Write(prompt);
        do  //loops as long as input is not between min and max
        {
            while (!int.TryParse(Console.ReadLine(), out number)) //loops as long as the input is not an integer
            {
                Console.Write("\nYou must enter an integer number!: ");
            }
            if (number < min || number > max) Console.Write(inputPrompt); //<--| write input prompt if number out of range
        } while (number < min || number > max);

        return number;
    }

    static void printGrade(string grade)
    {
        Console.WriteLine("\nYour potato is grade " + grade);
    }

    static void Main()
    {

        do
        {
            int weight = GetIntNumber(0, 1500, "\nEnter the weight of the potato in grams: ", "\nEnter an integer between 0 and 1500: ");

            if (weight < 201)
            {
                printGrade("X");
            }
            else
            {
                if (weight < 401)
                {
                    printGrade("A");
                }
                else
                {
                    if (weight < 801)
                    {
                        printGrade("B");
                    }
                    else
                        printGrade("Z");
                }
            }
            Console.WriteLine("\nPress any key to check the grade of another potato.");
        } while (Console.ReadKey(true).Key != ConsoleKey.Escape );

        Console.Read();

    }
}

Answer 1

You are very much correct that switch statements cannot take boolean operators like that: switch statements are used to build jump tables, and can only use constants to do so. However, you can easily come up with a fairly robust solution to your problem using a Dictionary and some LINQ.

Since you tagged your question beginner I'll be gentle with the explanation.

The first thing we'll do is make a Dictionary<int, string> which represents the lowest value associated with each grade.

var gradeMap = new Dictionary<int, string>
{
    [0] = "X",
    [201] = "A",
    [401] = "B",
    [801] = "Z",
};

So, this means if the value is >= 801, then it's Z, and so on.

Next, we'll simply one-line this entire operation in LINQ:

gradeMap.OrderByDescending(x => x.Key).FirstOrDefault(x => x.Key <= testWeight).Value

---

What's happening here? LINQ is Language Integrated Query, if you've ever worked with SQL you might recognize the Query bit, essentially, LINQ allows you to build code which looks and act's like a query against an IEnumerable object (which Dictionary is).

So if we take this step-by-step, we can start with our dictionary as follows:

0: X
201: A
401: B
801: Z

The gradeMap.OrderByDescending(x => x.Key) bit will first take the entire gradeMap dictionary, and order it by the key value in highest-to-lowest order:

801: Z
401: B
201: A
0: X

Next, we do a FirstOrDefault(x => x.Key <= testWeight), which will loop through all the elements (in the backwards order we have them) that are in the dictionary, and find the first one that matches our expression (x.Key <= testWeight). The x.Key is the reference to the key of the dictionary (our int values):

801: Z (Skipped)
401: B (Skipped)
201: A (Matched)

So, now we have the 201: A item from the dictionary, the last part is to get .Value on it which returns the string portion of it:

A

And viola, we have our value. :)

---

We can combine this together and get:

var testWeight = 205;
var grade = gradeMap.OrderByDescending(x => x.Key).FirstOrDefault(x => x.Key <= testWeight).Value;

So, the final thing we have to do is combine it with your original programme:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

class Program
{
    static int GetIntNumber(int min, int max, string prompt, string inputPrompt)
    {
        int number;

        Console.Write(prompt);
        do  //loops as long as input is not between min and max
        {
            while (!int.TryParse(Console.ReadLine(), out number)) //loops as long as the input is not an integer
            {
                Console.Write("\nYou must enter an integer number!: ");
            }
            if (number < min || number > max) Console.Write(inputPrompt); //<--| write input prompt if number out of range
        } while (number < min || number > max);

        return number;
    }

    static void printGrade(string grade)
    {
        Console.WriteLine("\nYour potato is grade " + grade);
    }

    static void Main()
    {
        do
        {
            var gradeMap = new Dictionary<int, string>
            {
                [0] = "X",
                [201] = "A",
                [401] = "B",
                [801] = "Z",
            };

            int weight = GetIntNumber(0, 1500, "\nEnter the weight of the potato in grams: ", "\nEnter an integer between 0 and 1500: ");

            printGrade(gradeMap.OrderByDescending(x => x.Key).FirstOrDefault(x => x.Key <= weight).Value);

            Console.WriteLine("\nPress any key to check the grade of another potato.");
        } while (Console.ReadKey(true).Key != ConsoleKey.Escape );

        Console.Read();
    }
}

The next best thing would be to extract the LINQ logic into a new method:

public string GetGrade(int weight)
{
    var gradeMap = new Dictionary<int, string>
    {
        [0] = "X",
        [201] = "A",
        [401] = "B",
        [801] = "Z",
    };

    return gradeMap.OrderByDescending(x => x.Key).FirstOrDefault(x => x.Key <= weight).Value;
}

Then our Main method becomes:

static void Main()
{
    do
    {
        int weight = GetIntNumber(0, 1500, "\nEnter the weight of the potato in grams: ", "\nEnter an integer between 0 and 1500: ");

        printGrade(GetGrade(weight));

        Console.WriteLine("\nPress any key to check the grade of another potato.");
    } while (Console.ReadKey(true).Key != ConsoleKey.Escape );

    Console.Read();
}

Now if you need to add a new grade, for whatever reason, it's trivial.

---

Also: if you're not using C#6.0, you will have to replace var gradeMap with the following:

var gradeMap = new Dictionary<int, string>
{
    {0, "X"},
    {201, "A"},
    {401, "B"},
    {801, "Z"}
};

Good work so far, hopefully you continue improving and learn more regarding C# and the paradigms it holds. :)

---

The answer by t3chb0t is also very good, I recommend reading it thoroughly as well (especially the bit about method overloading).

Answer 2

You can put else if together.
I doubt that this is any faster, but to me it is cleaner and I like even numbers

if (weight <= 200)
{
    printGrade("X");
}
else if (weight <= 400)
{
    printGrade("A");
}
else if (weight <= 800)
{
    printGrade("B");
}
else
{
    printGrade("Z");
}

Answer 3

What can be improved?

You can improve your code in several places. Let's start with GetIntNumber.

• Use helper variables for repeated conditions.
• Use helper variables to give a condition a meaning. It's better then writing comments.
• Use positive conditions and negate them if necessary. They're usually easier to understand.

With good helper variables you will not need comments, because now the variable names already explain how long the while is going to run. The improved code could look like this:

static int GetIntNumber(int min, int max, string prompt, string inputPrompt)
{
    int number;
    Console.Write(prompt);
    bool inputInRange;

    do
    {
        while (!int.TryParse(Console.ReadLine(), out number)) //loops as long as the input is not an integer
        {
            Console.Write("\nYou must enter an integer number!: ");
        }

        inputInRange = number >= min && number <= max;

        if (!inputInRange)
        {
            Console.Write(inputPrompt);
        }
    } while (!inputInRange);

    return number;
}

---

Method overloading

Now, let's do something about the Main, it currently contains important logic.

You can create another PrintGrade method, but this one will require an int. It will encapsulate the print-grade logic and use returns to avoid nesting or ugly if/elses.

Other advice:

• Use PascalCase for methods.
• Use constants instead of magic numbers.

Example:

static class WeightGradeMaxValue
{
    public const int X = 200;
    public const int A = 400;
    public const int B = 800;
    public const int Z = -1; // -1 might indicate no limit
}

static void PrintGrade(int weight) 
{

    if (weight <= WeightGradeMaxValue.X)
    {
        PrintGrade("X");
        return;
    }

    if (weight <= WeightGradeMaxValue.A)
    {
        PrintGrade("A");
        return;
    }

    if (weight <= WeightGradeMaxValue.B)
    {
        PrintGrade("B");
        return;
    }

    PrintGrade("Z");
}

Your Main is now nicely short:

static void Main()
{
    do
    {
        int weight = GetIntNumber(0, 1500, "\nEnter the weight of the potato in grams: ", "\nEnter an integer between 0 and 1500: ");

        PrintGrade(weight);

        Console.WriteLine("\nPress any key to check the grade of another potato.");
    } while (Console.ReadKey(true).Key != ConsoleKey.Escape);

    Console.Read();
}

---

More advanced solution

Even with all the suggested improvements there is still room for more.

The grade weights and symbols can be encapsulated in a new type:

class Grade
{
    public Grade(string symbol, int minWeight) 
    {
        Symbol = symbol;
        MinWeight = minWeight;
    }

    public string Symbol { get; }

    public int MinWeight { get; }

    public bool Includes(int weight)
    {
        return weight > MinWeight;
    }   
}

This new type is now able to check if a weight is within its range. This will help you to search for a particular grade.

Positive logic is usually esier to understand so I flipped it. Instead of having a max-weight I turned it into a min-weight so that we can use all grades and the logic becomes more obvious because you now check if the weight is in this grade a not if it's not in the other.

Creating new grades is now very easy and we pre-sort the array in descending order by MinWeight so that you don't have to do it each time.

private static readonly Grade[] Grades = new Grade[]
{
    new Grade("X", 0),
    new Grade("A", 200),
    new Grade("B", 400),
    new Grade("Z", 800),
}
.OrderByDescending(x => x.MinWeight)
.ToArray();

Searching for a grade is now as simple as to write a single LINQ expression - it's also easy to read because the logic is encapsulated in the Includes method:

static Grade GetGrade(int weight)
{
    return Grades.FirstOrDefault(x => x.Includes(weight)) ?? Grades.Last();
}

where the ?? (the null-coalescing operator) means that if the first part returns null in case nothing were found then use the second part or more technically:

It returns the left-hand operand if the operand is not null; otherwise it returns the right hand operand.

The last step is to exchange the old

PrintGrade(weight);

for the new one

PrintGrade(GetGrade(weight).Symbol);

Answer 4

A few suggestions:

• Why calculate it twice (number < min || number > max)
• You can avoid the if-else if you extract a formula for the requirement i.e if (weight-1)/200 == 0 => 'X'
  == 1 => 'A'
  == 2 => 'B'
  ==3 =>'B'
  others =='Z'

So, you can have an array

grades[5] = {'X','A','B','B','Z'}
factor = (weight-1)/200;
if(factor>=4) { factor = 4;}
return grade[factor];

I know its a bit incomprehensible but its short :)

• Other option is switch cases

Just realized a simpler solution. - Use a Dictionary with keys representing the upper limit of each range and values having the value for those range i.e

   valuesForRange[200] = 'X';
   valuesForRange[400] = 'A';
   valuesForRange[800] = 'B';
   valuesForRange[1500] = 'Z';

Loop through the keys (like an array) and wherever it matches, get the value.

NOTE: You will be misusing dictionary to act like a 2D array for different datatypes.