Max
Max

Reputation: 57

is there a better way to write this code for java? or to make it cleaner?

i wrote this code as part of a project and this doesnt seem the most efficient. is there a cleaner way to write this method?

public static int numberMonth(int parseMonth, Boolean leapYear)
        {
            int month = 0;
            if (parseMonth < 1)
            { month = 0;
                if (parseMonth < 2)
                { month =+ 31;
                    if (parseMonth < 3)
                    {
                        if (leapYear)
                        {
                        month =+ 29;
                        }
                        else if(!(leapYear))
                        {
                            month=+28;
                            if (parseMonth < 4)
                            {
                                month =+ 30;
                                if (parseMonth < 5)
                                {


                                    month =+ 31;
                                if (parseMonth < 6)
                                {
                                    month =+ 31;
                                    if (parseMonth < 7)
                                    {
                                        month =+ 30;
                                        if (parseMonth < 8)
                                        {
                                            month =+ 31;
                                            if (parseMonth < 9)
                                            {
                                                month =+ 31;
                                                if (parseMonth < 10)
                                                {
                                                    month =+ 30;
                                                    if (parseMonth < 11)
                                                    {
                                                        month =+ 31;
                                                        if (parseMonth < 12)
                                                        {
                                                            month =+31;
                                                        }
                                                    }
                                                }
                                            }
                                        }
                                    }
                                }
                            }
                        }
                    }
                }
            }
          }

Upvotes: 2

Views: 233

Answers (3)

jonmorgan
jonmorgan

Reputation: 2610

Your original code has a lot of problems. It doesn't return a value (you obviously intend to return month, but the compiler doesn't know that. It won't reach any of the code you want it to. There are other issues too, and while they won't keep your code from working, they will keep anybody from understanding it. What does parseMonth mean? What does leapYear mean? Why can a variable called month contain values far greater than the number of months in a year? There aren't any comments to explain any of this.

If I were writing this function, I would write the following (based on AndyMac's code with some slight modifications):

public static int numberOfDaysBeforeMonth(int monthNumber, boolean leapYear)
{
    //if monthNumber is out of range, return -1
    if(monthNumber< 1 || monthNumber > 12)
         return -1;

    int[] daysPerMonth= {31,28,31,30,31,30,31,31,30,31,30,31};

    int numberOfDays = 0;

    //add up the days in the months preceding the month in question
    for (int month = 1; month < monthNumber; month++)
       numberOfDays += daysPerMonth[month - 1];

    //add an extra day if it was a leap year and the month is after February
    if (leapYear && monthNumber > 2)
        numberOfDays++;

    return numberOfDays;
}

Upvotes: 1

bestsss
bestsss

Reputation: 12056

There you go and by the way, the code just doesn't work. The impl. is based on so if parseMonth = 4 then month = 31+28+31.

The idea is the simplest you can get, don't sum anything on the runtime, do it once during the initialization. You can init days[] by hand but I hope it's clear now.

package t1;

import java.util.Calendar;
import java.util.GregorianCalendar;

public class MonthyDays {
    static final int[] days;
    static{
        int first = 1;//should be zero for decent logic
        days=new int[first+Calendar.UNDECIMBER];

        GregorianCalendar c=new GregorianCalendar();
        c.set(1999, 0, 1, 0, 0,0);//use some non leap year      
        for (int i=Calendar.JANUARY;i<Calendar.UNDECIMBER;i++){
            days[first+i] = c.get(Calendar.DAY_OF_YEAR)-1;
            c.add(Calendar.MONTH, 1);
        }
    }
    public static int numberMonth(int parseMonth, String leapYear){
        if (parseMonth<=0 || parseMonth>12)
            return 0;//should be IndexOutOfBounds; i.e. the chech should be removed;

        int day = days[parseMonth];
        if (parseMonth>2 && "leap".equals(leapYear))//avoid NPE
            day++;
        return day;
    }

    public static void main(String[] args) {
        System.out.println(numberMonth(2, "leap"));
        System.out.println(numberMonth(3, "leap"));

        System.out.println(numberMonth(1, "leap"));
        System.out.println(numberMonth(1, "leap"));

        System.out.println(numberMonth(12, ""));
        System.out.println(numberMonth(12, "leap"));

    }
}

Upvotes: 0

AndyMac
AndyMac

Reputation: 830

codaddict, I think that's almost right. I think he's trying to add days in the year, so you just need to throw your days variable into a loop:

public static int numberMonth(int parseMonth, String leapYear)
{
    if(parseMonth<1 || parseMonth>12) {
         return 0;
    }
    int[] monthArray = {31,28,31,30...};
    int days = monthArray[0];
    for (int ii = 1; ii < parseMonth; ii++) {
       days += monthArray[parseMonth];
    }
    if (leapYear.equals("leap") && parseMonth > 1) {
        days++;
    }
    return days;
}

But I think instead the OP should look at Calendar. Check a couple of the answers in this thread: Calculate days in year

Upvotes: 1

Related Questions