4
\$\begingroup\$

Given a number between 1 and 7, this will display which day of the week corresponds to that number, starting with Monday. This is pretty simple and pretty easy, but it seems that there would be a much better way to do this, where I wouldn't have to write out each day of the week, and possibly iterate through an array..?

using System;
namespace FindDay
{
 class Program
 {
 static void Main(string[] args)
 {
 //This program will provide you the day of the week
 //through a given number that coresponds to that day
 //of the week, IE: Monday => 1st day of the week
 int num;
 string result;
 label:
 Console.Write("Enter a number between 1 and 7: ");
 num = Convert.ToInt32(Console.ReadLine());
 switch (num)
 {
 case 1:
 result = "Monday";
 Console.WriteLine("{0} => {1}st day of the week", result, num);
 break;
 case 2:
 result = "Tuesday";
 Console.WriteLine("{0} => {1}nd day of the week", result, num);
 break;
 case 3:
 result = "Wednesday";
 Console.WriteLine("{0} => {1}rd day of the week", result, num);
 break;
 case 4:
 result = "Thursday";
 Console.WriteLine("{0} => {1}th day of the week", result, num);
 break;
 case 5:
 result = "Friday";
 Console.WriteLine("{0} => {1}th day of the week", result, num);
 break;
 case 6:
 result = "Saturday";
 Console.WriteLine("{0} => {1}th day of the week", result, num);
 break;
 case 7:
 result = "Sunday";
 Console.WriteLine("{0} => {1}th day of the week", result, num);
 break;
 default:
 Console.WriteLine("That's not a day..");
 goto label;
 }
 }
 }
}

Would it be better to iterate through an array within this instead of writing out each day separately, how can I improve this little program?

asked May 19, 2016 at 17:57
\$\endgroup\$
1
  • 6
    \$\begingroup\$ Any time you find yourself copy-pasting the same code over and over again, is a good time to ask yourself "should this be in a local variable? Should it be in a method? Should it be in a class?" These are all different ways to achieve re-use and lower repetition. \$\endgroup\$ Commented May 19, 2016 at 20:34

3 Answers 3

19
\$\begingroup\$

I would add to forsvarir's good answer: start thinking now about the power of abstraction. Programming is all about building abstractions. Make your abstractions solve one problem and solve it well. For instance, here's a problem to solve:

static string Ordinal(int n)
{
 // You fill this in. The method takes an integer and returns
 // 1st, 2nd, 3rd, 4th, etc. 
}

Heres another one:

static string WeekdayName(int w)
{
 // To do: Return the name of the day of the week.
}

And here's another:

static int GetIntegerFromConsole(int low, int high)
{
 // to do: prompt the user, sit in a loop until they give
 // a response in the right range, and return it.
}

Once you have those methods then your program becomes trivial.

int weekdayNumber = GetIntegerFromConsole(1, 7);
string weekdayName = WeekdayName(weekdayNumber);
string ordinal = Ordinal(weekdayNumber);
Console.WriteLine($"{weekdayName} => {ordinal} day of the week");

You know what I like? Four-line-long programs, that's what I like. Move the mechanisms to their own helper methods, debug those methods, and then rely on them to do their jobs.

answered May 19, 2016 at 20:42
\$\endgroup\$
0
6
\$\begingroup\$

You're correct that your program could make use of arrays to simplify it. Something like this achieves the same result:

var days = new string[] { "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", 
 "Saturday", "Sunday" };
var postfix = new string[] { "st", "nd", "rd", "th", "th", "th", "th" };
do {
 Console.Write("Enter a number between 1 and 7: ");
 num = Convert.ToInt32(Console.ReadLine());
 if (num < 1 || num > 7)
 {
 Console.WriteLine("That's not a day..");
 }
}
while(num < 1 || num > 7);
// Notice that since arrays start at 0, I'm indexing the selected number
// minus one
Console.WriteLine("{0} => {1}{2} day of the week", days[num-1], num, postfix[num-1]);

You are also going to want to think about putting some kind of error checking around this:

num = Convert.ToInt32(Console.ReadLine());

At the moment, if you put something like "some random string" it will get confused and throw an unhandled exception.

Generally speaking people don't like the use of labels (although this is obviously subjective), which is why I've used a do loop instead of goto label.

answered May 19, 2016 at 19:09
\$\endgroup\$
3
  • 2
    \$\begingroup\$ Another improvement: the "prompt, check for validity, give an error, loop until valid" pattern is crying out to be extracted to its own helper method: int result = Prompt(lower, upper, error); \$\endgroup\$ Commented May 19, 2016 at 20:33
  • \$\begingroup\$ Please replace your 7 with something like days.Length. And - but this is probably subject to taste - i'd handle the "array indexes start with 0" thing with an extra empty first element instead of num-1. \$\endgroup\$ Commented May 20, 2016 at 6:01
  • \$\begingroup\$ @GuntramBlohm Whilst a constant like 'numberOfDaysInWeek' is an option, in this instance I think the use of the numbers 1 & 7 is appropriate. The numbers are explicitly brought into the problem domain by the prompt given to the user asking for 'a number between 1 and 7'. For me, days.Length would reduce readability. It can be useful to have empty elements for some collection implementation algorithms, however again in this instance I think having an empty element at the beginning of a days array would reduce readability. \$\endgroup\$ Commented May 20, 2016 at 8:46
1
\$\begingroup\$

There are many different design-oriented ways to produce a good solution, as demonstrated in the other answers. So I'd like to suggest something different.

One should make use of framework classes whenever possible. It avoids potential bugs, reduces the lines of code and standardizes the code.

There is an enum in the System namespace called DayOfWeek. It has an ordered list of the days of the week. All you need to do is parse the enum as a string as detailed below

Console.Write("Enter a number between 1 and 7: ");
int inputNumber = Convert.ToInt32(Console.ReadLine());
// cast integer as enum and convert to string
string dayFromNumber = ((DayOfWeek)inputNumber).ToString();
Console.WriteLine("Day {0} of a week is {1}.", inputNumber, dayFromNumber);

It's a 4-line solution by just using available resources in .NET. You can further improve the implementation by introducing an IF for values between 1 and 7 and so on.

To improve the readability you can rephrase the main conversion as

string dayFromNumber = Enum.GetName(typeof(DayOfWeek), inputNumber);

and, this implementation even avoids the string to int conversion (does gets quite verbose though)

string dayFromNumber = Enum.Parse(typeof(DayOfWeek), inputString).ToString();
answered May 20, 2016 at 16:12
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Using DayOfWeek was also my first instinct. However, validating 1 <= n <= 7, translating between 0 and 7, and producing ordinal numbers are also significant parts of the exercise, so it turns out to be a minor improvement. \$\endgroup\$ Commented May 20, 2016 at 18:26

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.