Using a for loop when calling a method

Question

I'm using a for loop in a method to carry the result to the main function. I'm trying to use the for loop to get the month of a year and pass it on to the output from the main function.

I have nested an if loop in the for loop which I feel is probably redundant as the for loop will count to the end anyway. It's probably a basic enough problem in the code but I have been staring at it for so long that I think it's burn out on my end.

The output is returning "Doesn't Exist" for all months instead of picking out the relevant month. How do I pick out the relevant month from the for loop or is that possible with the way I have coded so far?

namespace Month_Function_Call
    {
class Program
{
    public static String month_name(int month)
    {
        String result;
        result = "a";
        for (int i = 0; i < 12; ++i )
        {
            if (i == 0)
            {
                result = "January";
            }
            if (i == 1)
            {
                result = "February";
            }
            if (i == 2)
            {
                result = "March";
            }
            if (i == 3)
            {
                result = "April";
            }
            if (i == 4)
            {
                result = "May";
            }
            if (i == 5)
            {
                result = "June";
            }
            if (i == 6)
            {
                result = "July";
            }
            if (i == 7)
            {
                result = "August";
            }
            if (i == 8)
            {
                result = "September";
            }
            if (i == 9)
            {
                result = "October";
            }
            if (i == 10)
            {
                result = "November";
            }
            if (i == 11)
            {
                result = "December";
            }
            else
            {
                result = "N/A";
            }


        }
            return result;
    }
    static void Main(string[] args)
    {
        Console.WriteLine("Month 1: " + month_name(1));
        Console.WriteLine("Month 2: " + month_name(2));
        Console.WriteLine("Month 3: " + month_name(3));
        Console.WriteLine("Month 4: " + month_name(4));
        Console.WriteLine("Month 5: " + month_name(5));
        Console.WriteLine("Month 6: " + month_name(6));
        Console.WriteLine("Month 7: " + month_name(7));
        Console.WriteLine("Month 8: " + month_name(8));
        Console.WriteLine("Month 9: " + month_name(9));
        Console.WriteLine("Month 10: " + month_name(10));
        Console.WriteLine("Month 11: " + month_name(11));
        Console.WriteLine("Month 12: " + month_name(12));
        Console.WriteLine("Month 43: " + month_name(43));
        Console.ReadKey();
    }
}

Why are you looping instead of just comparing `month` in the `if` statements? — juharr, Nov 23 '15 at 15:11
What problem are you encountering? What help are you actually seeking? If its code review you are after then codereview.stackexchange.com exists and might be a better place for the question (but only if your code actually works). — Chris, Nov 23 '15 at 15:11
You don't need a loop in your method, you need the loop in `Main` to pass month `1` to `12`. Also look into `Dictionary` so that you can get ride of the `if` statement in your method. Plus there are already methods available to get Month name based on number. One last thing is you need `if ... else ... if` to sort out invalid choice, you may use `switch` here as well. — Habib, Nov 23 '15 at 15:12
You compare against `0` for January, but start with `1` in your calling code. — crashmstr, Nov 23 '15 at 15:12
You should also replace the entire, if...if... into a simple array lookup. string []{"January", "February", ....} Then, you can just look for the index you want. — wandercoder, Nov 23 '15 at 15:14
http://stackoverflow.com/questions/3184121/get-month-name-from-month-number — Greg, Nov 23 '15 at 15:15
@Damo, your main problem is you have a method accepting an `int` representing the month, but you never reference that object. You method should use the information from its parameters in calculating a result. — Jonesopolis, Nov 23 '15 at 15:15
Also you need to clarify if you want a zero based or one based indexing of the months. In your code `if (i == 0) { result = "January"; }` indicate you want a 0 based indexing, but then in your `Main` you run it for 1-12. — juharr, Nov 23 '15 at 15:31

Magnus · Answer 1 · 2015-11-23T15:31:16.847

7

I think using GetMonthName from DateTimeFormat would be a better way of doing it. This will give you the name in the users active culture. (You can of course hard code this to any culture you like) Then ToTitleCase to get the first character as upper case.

public static String month_name(int month)
{
   if(month < 1 || month > 12) 
      return "N/A";
   var culture = CultureInfo.CurrentCulture;
   var name = culture.DateTimeFormat.GetMonthName(month);
   return culture.TextInfo.ToTitleCase(name);
}

edited Nov 23 '15 at 15:31

answered Nov 23 '15 at 15:17

Magnus

45,362
8
80
118

2

First the OP is using a zero based index for the month names and this uses a 1 based. Second this returns an empty string for a value of 13 and throws an exception for values less than 1 and greater than 13. – juharr Nov 23 '15 at 15:20
@juharr As for zero based index that seems to be wrong in his function if you look at his Main method it assumes 1 to be the first month. – Magnus Nov 23 '15 at 15:32
@juharr you're right, that was a mistake I spotted after you mentioned it, took me a while to figure out why I got N\A for two months instead of one but it was the index that was wrong – Damo Nov 23 '15 at 15:37
@Magnus Yeah I just noticed that too and asked the OP to clarify – juharr Nov 23 '15 at 15:37

score 5 · Accepted Answer · answered Nov 23 '15 at 15:19

You could make this cleaner by using switch

 switch (month)
        {
            case 0: return "January";
            case 1: return "February";
            case 2: return "March";
            case 3: return "April";
            case 4: return "May";
            case 5: return "June";
            case 6: return "July";
            case 7: return "August";
            case 8: return "September";
            case 9: return "October";
            case 10: return "November";
            case 11: return "December";
            default: return "N/A";
        }

score 4 · Answer 3 · edited Nov 23 '15 at 16:11

4

Do something like this instead

string[] months = new string[12] {"January", "February", "March" }; // Input all months to this array

if (index <= -1 || index > 12) return "N/A";
return months[index];

Insert this code into the getMonthName() function

edited Nov 23 '15 at 16:11

Kevin Cruijssen

9,153
9
61
135

answered Nov 23 '15 at 15:16

Yytsi

424
1
5
14

I saw your 43 test case, let me edit the answer that suits that, hmm. – Yytsi Nov 23 '15 at 15:17
Thank you that makes infinitely more sense than the nonsense I was typing! I knew it would be something simple that I missed – Damo Nov 23 '15 at 15:17
2

Just do `return index >= 0 && index < 12 ? months[index] : "N/A";` – juharr Nov 23 '15 at 15:18
1

@juharr You're right, that suits it better, I just made it like that so OP would read and understand what the code is performing. – Yytsi Nov 23 '15 at 15:22
1

@TuukkaX Yeah, I actually added that comment before you added the `if` though you current have the logic backwards. – juharr Nov 23 '15 at 15:33

score 3 · Answer 4 · answered Nov 23 '15 at 15:19

Dont use loop and if-else. What you need is dictionary.

static Dictionary<int, string> _monthName = new Dictionary<int, string>
{
    {1,"January" }, // if zero based start from 0
    {2,"February" },
    {3,"March" },
    {4,"April" },
    {5,"May" },
    {6,"June" },
    {7,"July" },
    {8,"August" },
    {9,"September" },
    {10,"October" },
    {11,"November" },
    {12,"December" },
};

private static string GetMonthName(int i)
{
    var result = "";
    if (_monthName.TryGetValue(i, out result))
    {
        return result;
    }
    return "N/A";
}
static void Main(string[] args)
{
    Console.WriteLine("Month 1: " + GetMonthName(1));
    Console.WriteLine("Month 2: " + GetMonthName(2));
    Console.WriteLine("Month 3: " + GetMonthName(3));
    Console.WriteLine("Month 4: " + GetMonthName(4));
    Console.WriteLine("Month 5: " + GetMonthName(5));
    Console.WriteLine("Month 6: " + GetMonthName(6));
    Console.WriteLine("Month 7: " + GetMonthName(7));
    Console.WriteLine("Month 8: " + GetMonthName(8));
    Console.WriteLine("Month 9: " + GetMonthName(9));
    Console.WriteLine("Month 10: " + GetMonthName(10));
    Console.WriteLine("Month 11: " + GetMonthName(11));
    Console.WriteLine("Month 12: " + GetMonthName(12));
    Console.WriteLine("Month 43: " + GetMonthName(43));
    Console.ReadKey();
}

score 1 · Answer 5 · answered Nov 23 '15 at 15:29

In my opinion You should ommit using so many if statements in the for loop in this case because it is not readable enough and the better approach would be to create an array of type string which will have all the names of the months and to iterate this array. Your code would be like:

public static String month_name(int month) {
        String result;
        result = "a";
        // for the sake of readability I have split the line
        String[] allMonths = { 
                              "N/A", "January", "February", "March", "April",
                              "May", "June", "July", "August", "September", 
                              "October", "November", "December" 
                          };
        if (month >= 0 && month <= 12)
            result = allMonths[month];
        else
           result = "N/A";

        return result;
    }

    static void Main(string[] args) {
        Console.WriteLine("Month 1: " + month_name(1));
        Console.WriteLine("Month 2: " + month_name(2));
        Console.WriteLine("Month 3: " + month_name(3));
        Console.WriteLine("Month 4: " + month_name(4));
        Console.WriteLine("Month 5: " + month_name(5));
        Console.WriteLine("Month 6: " + month_name(6));
        Console.WriteLine("Month 7: " + month_name(7));
        Console.WriteLine("Month 8: " + month_name(8));
        Console.WriteLine("Month 9: " + month_name(9));
        Console.WriteLine("Month 10: " + month_name(10));
        Console.WriteLine("Month 11: " + month_name(11));
        Console.WriteLine("Month 12: " + month_name(12));
        Console.WriteLine("Month 43: " + month_name(43));
        Console.ReadKey();
    }

Hope it helped :)

score 0 · Answer 6 · answered Nov 23 '15 at 16:43

0

Using the Swith-Case statement is the best choice here.

But, if you don't know how to use it, you should remove the For statement, because it is completely non-sense.

 //for (int i = 0; i < 12; ++i )
    //{

    //}

answered Nov 23 '15 at 16:43

alessalessio

1,224
1
15
28

Using a for loop when calling a method

6 Answers6