Reputation: 57
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
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
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
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