Calculating sum and count of even and odd numbers (follow-up)

Question

(This is follow-up question). I updated my console program code to calculate sum and count of even and odd numbers. Provided code :

using System;
namespace Studying_
{
 class Program
 {
 static void Main(string[] args)
 {
 uint evenNumbersCount = 0;
 uint oddNumbersCount = 0;
 int rangeStartInclusive, rangeEndInclusive;
 long sum;
 while (true)
 {
 Console.WriteLine("Enter the first number in the range to find out it odd or even.");
 if (int.TryParse(Console.ReadLine(), out rangeStartInclusive) == false)
 {
 Console.WriteLine("Incorrect number. Press R to try again. Press any key to exit.");
 if (Console.ReadKey().KeyChar == 'r' || Console.ReadKey().KeyChar == 'R')
 {
 Console.Clear();
 continue;
 }
 else
 break;
 }
 Console.WriteLine("Enter the second number");
 if (int.TryParse(Console.ReadLine(), out rangeEndInclusive) == false)
 {
 Console.WriteLine("Incorrect number. Press R to try again. Press any key to exit.");
 if (Console.ReadKey().KeyChar == 'r' || Console.ReadKey().KeyChar == 'R')
 {
 Console.Clear();
 continue;
 }
 else
 break;
 }
 if (rangeEndInclusive < rangeStartInclusive)
 (rangeStartInclusive, rangeEndInclusive) = (rangeEndInclusive, rangeStartInclusive);
 for (int n = rangeStartInclusive; n <= rangeEndInclusive; n++)
 {
 bool isEven = n % 2 == 0;
 if (isEven)
 evenNumbersCount++;
 else
 oddNumbersCount++;
 }
 sum = (rangeEndInclusive - rangeStartInclusive + 1) * (rangeStartInclusive + rangeEndInclusive) / 2;
 Console.WriteLine("Sum - " + sum + " | Even - " + evenNumbersCount + " | Odd - " + oddNumbersCount);
 Console.WriteLine("Press R to try again. Press any key to exit.");
 char key = Console.ReadKey().KeyChar;
 if (key == 'r' || key == 'R')
 {
 evenNumbersCount = 0;
 oddNumbersCount = 0;
 Console.Clear();
 continue;
 }
 else
 break;
 }
 }
 }
}

Input: 2, 5
Output: Sum - 14 | Even - 2 | Odd - 2
(2,3,4,5) counts.

The question is whether I can improve my code. I have some doubts about this if with upper- and lowercase. Are there some ways to improve it?

if (Console.ReadKey().KeyChar == 'r' || Console.ReadKey().KeyChar == 'R')

Also, too many messages like

Console.WriteLine("Press R to try again. Press any key to exit.");

Maybe I should initialize method Exit() or NewTry() (Haven't gotten to the methods yet, but I once studied them). The code will be more readable, but will it affect on performance? Expecting for your feedbacks.

P.S. Sorry for my english skills.

• Peter Csala: From what perspective do you want improvement(s)?
• JediMan: I guess from any one. I want people to share with me if I can make something simpler or in a better way.
• Peter Csala: I think you misunderstood my question. Which quality attribute needs improvement?
• aepot: if (Console.ReadKey().Key == ConsoleKey.R)
• JediMan: @aepot, thanks, this is the most easier way given here.

Answer 1

I like that you use Inclusive as part of some names because it adds to clarity. When I see n <= rangeEndInclusive there is no doubt that it correctly used, whereas n < rangeEndInclusive or even n <= rangeEndInclusive would raise doubts.

I can appreciate you are a beginner and willing to learn. To that end, one does not typically end object names with an underscore. So namespace Studying_ should just be namespace Studying.

Here at CR, we have a strong preference to always use { }. You have several instance with conditionals where you do not do this.

I would not use uint for evenNumbersCount and oddNumbersCount. The input values are int, so pretty sure you will never exceed an int. While they never will they be less than 0, they should still be declared as int.

Declare one variable per line. So break int rangeStartInclusive, rangeEndInclusive; into 2 different lines.

It is probably correct that sum is a long since you could specify a number from 1 to int.MaxValue. Just wait a while to have that calculate since it's 2.1 billion values.

One coding principle to adopt as your own personal philosophy is Don't Repeat Yourself or DRY. Try to keep your code DRY. When you prompt for the 2 numbers, much of that code is repetitive. Try to see if you can rework it into its own method. I leave that as an exercise to you.

An alternative to counting odds and evens:

int[] count = new int[2];
for (int n = rangeStartInclusive; n <= rangeEndInclusive; n++)
{
 count[n % 2]++;
}
Console.WriteLine($"Odd count = {count[1]}");
Console.WriteLine($"Even count = {count[0]}");

• JediMan: Thank you, I think this answer is the most valuable. But I cannot understand what { } preference do you mean? Do you mean I should add { } in if (isEven) evenNumbersCount++; else oddNumbersCount++; ?
• Rick Davin: @JediMan For if or for blocks, CR prefers to use { } rather than just 1 line. The one code block I posted has an example. The braces could have been omitted, but it is considered a good practice to always use them in case you ever modify the code by adding extra lines.

Answer 2

Review

Welcome to Code Review. There are few suggestions as below.

string interpolation

As some answers mentioned in the question you linked, you can use string interpolation and Console.WriteLine("Sum - " + sum + " | Even - " + evenNumbersCount + " | Odd - " + oddNumbersCount); is going to be Console.WriteLine($"Sum - {sum} | Even - {evenNumbersCount} | Odd - {oddNumbersCount}");

Create methods

DRY - Don't repeat yourself. The part of reading the first number and the second number are similar. Moreover, you write three times if (Console.ReadKey().KeyChar == 'r' || Console.ReadKey().KeyChar == 'R') You can create static methods (for example, GetNumber and CheckExit here) to do these things.

static void Main(string[] args)
{
 uint evenNumbersCount = 0;
 uint oddNumbersCount = 0;
 int rangeStartInclusive, rangeEndInclusive;
 long sum;
 while (true)
 {
 Console.WriteLine("Enter the first number in the range to find out it odd or even.");
 if (GetNumber(out rangeStartInclusive) == false) continue;
 Console.WriteLine("Enter the second number");
 if (GetNumber(out rangeEndInclusive) == false) continue;
 if (rangeEndInclusive < rangeStartInclusive)
 (rangeStartInclusive, rangeEndInclusive) = (rangeEndInclusive, rangeStartInclusive);
 for (int n = rangeStartInclusive; n <= rangeEndInclusive; n++)
 {
 bool isEven = n % 2 == 0;
 if (isEven)
 evenNumbersCount++;
 else
 oddNumbersCount++;
 }
 sum = (rangeEndInclusive - rangeStartInclusive + 1) * (rangeStartInclusive + rangeEndInclusive) / 2;
 Console.WriteLine($"Sum - {sum} | Even - {evenNumbersCount} | Odd - {oddNumbersCount}");
 CheckExit("Press R to try again. Press any key to exit.");
 evenNumbersCount = 0;
 oddNumbersCount = 0;
 Console.Clear();
 
 }
}
private static bool GetNumber(out int result)
{
 if (int.TryParse(Console.ReadLine(), out result) == false)
 {
 return CheckExit("Incorrect number. Press R to try again. Press any key to exit.");
 }
 return true;
}
private static bool CheckExit(string message)
{
 Console.WriteLine(message);
 if (Console.ReadKey().KeyChar == 'r' || Console.ReadKey().KeyChar == 'R')
 {
 Console.Clear();
 return false;
 }
 else
 Environment.Exit(0);
 return true;
}

Answer 3

for your question :

if (Console.ReadKey().KeyChar == 'r' || Console.ReadKey().KeyChar == 'R')

it can be simplified using StringComparison to ignore case:

if(Console.ReadLine().Equals("r", StringComparison.OrdinalIgnoreCase))
{
 Environment.Exit(0);
}

Also, your user input validation is a good start, but still need a better handling. As the current approach will invalidate the results or throw some exceptions if either inputs is invalid.

To improve this approach, you need to ensure that your application always gets the correct value from the user, and prevent the application from moving to the next step if it's invalid. This means, on every input, you'll put the user in a loop to validate the given input, if the input is invalid, then notify the user to get a new input or exit. So, each step will either pass the validation or exit the application by the user.

Example :

Console.WriteLine("Enter the first number in the range to find out it odd or even.");
while(int.TryParse(Console.ReadLine(), out rangeStartInclusive) == false)
{
 Console.WriteLine("Invalid input, only integers are expected. Please enter an integer number OR Press R to exit.");
 if(Console.ReadLine().Equals("r", StringComparison.OrdinalIgnoreCase))
 {
 Environment.Exit(0);
 } 
}

Instead of asking the user to enter R to continue, you need to ask the user to input R to exit (or whatever letter you like). This would forces the user to enter only correct inputs (take it or leave situation). As you need to narrow down user freedom to control the inputs and avoid any unhandled cases and to minimize code vulnerabilities (play it safe). (this is something you should keep in mind as it would increase your code security awareness over the time).

Now, you can take this loop and move it to another method to reuse it on each user input.

example :

private static int GetUserInputAsInt(string message) 
{
 int result = 0;
 
 Console.WriteLine(message);
 
 while(int.TryParse(Console.ReadLine(), out result) == false)
 {
 Console.WriteLine("Invalid input, only integers are expected. Please enter an integer number OR Press R to exit.");
 if(Console.ReadLine().Equals("r", StringComparison.OrdinalIgnoreCase))
 {
 Environment.Exit(0);
 } 
 }
 return result;
}

then you can simply do this :

int rangeStartInclusive = GetUserInputAsInt("Enter the first number in the range to find out it odd or even.");
int rangeEndInclusive = GetUserInputAsInt("Enter the second number");

Now, you do a method for counting the odds and evens and summing!, and since you're working with mathematical scope, you will also need to do more researching about it, to simplify your work more further.

For instance, you don't need a loop to count the odds and evens, there is a mathematical pattern (not my greatest subject), not sure if there is a solid formula that can be used, but from what I can see you can determine the number of odds by subtracting the numbers and dividing them by 2.

Example :

input : 10,50 
Odds = 50 -ひく 10 =わ 40 =わ 40 / 2 = 20
Sum = 10 + 20 = 30 x 20 = 600
---------------
input : 22,30 
Odds = 30 - 22 = 8 = 8 / 2 = 4
Sum = 22 + 4 = 26 x 4 = 104
---------------
// with odds inputs
input : 1,10 
Odds = 10 - 1 = 9 = (9 - 1) / 2 = 4 + 1 = 5
Sum = 1 + 9 = (10 / 2) x 5 = 25
---------------
input : 11,30 
Odds = 30 - 11 = 19 = (19 - 1) / 2 = 9 + 1 = 10
Sum = 11 + 19 = (30/ 2) x 10 = 150

So, we can use this to our advantage and elimnate the need of the loops by doing something like :

public static int CountOddNumbers(int leftHand, int rightHand)
{
 if(leftHand == 0 && rightHand == 0) { return 0; }
 int subtractHands = Math.Abs(leftHand - rightHand);
 if(subtractHands == 0) { return 0; }
 int count = subtractHands / 2;
 if(count == 0) { return 1; }
 if(leftHand % 2 != 0 || rightHand % 2 != 0) { count++; }
 return count;
}
public static int SumOddNumbers(int leftHand, int rightHand)
{
 int count = CountOddNumbers(leftHand, rightHand);
 int sum = Math.Min(leftHand , rightHand) + count;
 
 if(leftHand % 2 != 0 || rightHand % 2 != 0) { count++; }
 sum *= count;
 
 return sum; 
}

for the even numbers, I found that the count of odd numbers is always equal to even numbers, not sure if there is edge cases to this, but we can do this :

public static int CountEvenNumbers(int leftHand, int rightHand)
{
 int odds = CountOddNumbers(leftHand, rightHand);
 int subtractHands = Math.Abs(leftHand - rightHand) + 1;
 return subtractHands - odds;
}

Now, revising your code would be something like this :

public static void Main(string[] args)
{
 int rangeStartInclusive = GetUserInputAsInt("Enter the first number in the range to find out it odd or even.");
 
 int rangeEndInclusive = GetUserInputAsInt("Enter the second number");
 
 int oddNumbersCount = CountOddNumbers(rangeStartInclusive, rangeEndInclusive);
 
 int evenNumbersCount = CountEvenNumbers(rangeStartInclusive, rangeEndInclusive);
 
 int sum = SumOddNumbers(rangeStartInclusive, rangeEndInclusive);
 
 Console.WriteLine($"Sum: {sum} | Even: {evenNumbersCount} | Odd: {oddNumbersCount}");
}

• JediMan: Thank you, I know math and thought about this realization. It would be much faster, you're right.

Answer 4

If you ever get interested in converting the code from procedural to object-oriented, here are some thoughts on an object model (excluding the user input piece):

public class Number
{
 public int Value { get; private set; } 
 public bool IsEven { get; private set; }
 public bool IsOdd { get; private set; }
 
 public Number(int i) 
 {
 Value = i;
 IsEven = Value % 2 == 0;
 IsOdd = !IsEven;
 }
}
public class Numbers
{
 public List<Number> Items { get; private set; }
 public Numbers(List<Number> items) => Items = items;
 public int Total() => Items.Sum(n => n.Value);
 public List<Number> Odds() => Items.Where(i => i.IsOdd).ToList();
 public List<Number> Evens() => Items.Where(i => i.IsEven).ToList();
 public override string ToString() =>
 $"Sum: {Total():N0} | Evens: {Evens().Count:N0} | Odds: {Odds().Count:N0}";
}
public class App
{
 public void Run()
 {
 var range = Enumerable.Range(1, 10).Select(i => new Number(i)).ToList();
 var numbers = new Numbers(range);
 Console.WriteLine(numbers.ToString());
 }
}
class Program 
{
 static void Main()
 {
 var app = new App();
 app.Run();
 }
}

• Rick Davin: Feels like overkill to determine odd or even. Plus your IsEven and IsOdd should be class properties instead of methods.
• Aron: Good point, I switched IsEven and IsOdd to properties.
• JediMan: This is good, but not quite my level yet. What is the advantage in converting code from procedural to object-oriented?
• Aron: Good question. While you can write procedural code in it, C# is an object-oriented language, so it's best-suited to writing object-oriented code. Object-oriented programming is its own discipline, subject to many holy wars about its validity and usefulness.