patternjavaModerate
Day of year calculation method
Viewed 0 times
calculationdayyearmethod
Problem
The exercise I wanted to solve is from here. Copying from that page:
This looks like a trivial example but it took me some time to follow the 'best practices' such as immutable objects, SOLID principles, etc.. And I am not sure if I was successful in following them.
I advise you to give you a try yourself actually!
And I wanted to refactor this "smelly" example, and here is my implementation in Java.
Months are hardcoded with number of days they have:
```
public enum Month {
JAN(1, 31, 31),
FEB(2, 28, 29),
MAR(3, 31, 31),
APR(4, 30, 30),
MAY(5, 31, 31),
JUN(6, 30, 30),
JUL(7, 31, 31),
AUG(8, 31, 31),
SEP(9, 30, 30),
OCT(10, 31, 31),
NOV(11, 30, 30),
DEC(12, 31, 31);
private final int monthIndex;
private final int numberOfDaysInNonLeapYear;
private final int numberOfDaysInLeapYear;
Month(int monthIndex, int numberOfDaysInNonLeapYear, int numberOfDaysInLeapYear) {
this.monthIndex = monthIndex;
this.numberOfDaysInNonLeapYear = number
public static int dayOfYear(int month, int dayOfMonth, int year) {
if (month == 2) {
dayOfMonth += 31;
} else if (month == 3) {
dayOfMonth += 59;
} else if (month == 4) {
dayOfMonth += 90;
} else if (month == 5) {
dayOfMonth += 31 + 28 + 31 + 30;
} else if (month == 6) {
dayOfMonth += 31 + 28 + 31 + 30 + 31;
} else if (month == 7) {
dayOfMonth += 31 + 28 + 31 + 30 + 31 + 30;
} else if (month == 8) {
dayOfMonth += 31 + 28 + 31 + 30 + 31 + 30 + 31;
} else if (month == 9) {
dayOfMonth += 31 + 28 + 31 + 30 + 31 + 30 + 31 + 31;
} else if (month == 10) {
dayOfMonth += 31 + 28 + 31 + 30 + 31 + 30 + 31 + 31 + 30;
} else if (month == 11) {
dayOfMonth += 31 + 28 + 31 + 30 + 31 + 30 + 31 + 31 + 30 + 31;
} else if (month == 12) {
dayOfMonth += 31 + 28 + 31 + 30 + 31 + 30 + 31 + 31 + 30 + 31 + 31;
}
return dayOfMonth;
}This looks like a trivial example but it took me some time to follow the 'best practices' such as immutable objects, SOLID principles, etc.. And I am not sure if I was successful in following them.
I advise you to give you a try yourself actually!
And I wanted to refactor this "smelly" example, and here is my implementation in Java.
Months are hardcoded with number of days they have:
```
public enum Month {
JAN(1, 31, 31),
FEB(2, 28, 29),
MAR(3, 31, 31),
APR(4, 30, 30),
MAY(5, 31, 31),
JUN(6, 30, 30),
JUL(7, 31, 31),
AUG(8, 31, 31),
SEP(9, 30, 30),
OCT(10, 31, 31),
NOV(11, 30, 30),
DEC(12, 31, 31);
private final int monthIndex;
private final int numberOfDaysInNonLeapYear;
private final int numberOfDaysInLeapYear;
Month(int monthIndex, int numberOfDaysInNonLeapYear, int numberOfDaysInLeapYear) {
this.monthIndex = monthIndex;
this.numberOfDaysInNonLeapYear = number
Solution
Your code is way too bloated. The definitions for leap years will not change in the foreseeable future, so this simple utility method does not need to apply to changing requirements.
This is all that is needed. 6 lines instead of 84. So when there should ever be a bug in the code, it will be easy to find, since there are only 3 lines where it could hide.
private static final int[] DAYS_BEFORE = {0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334};
public static int dayOfYear(int month, int dayOfMonth, int year) {
int leapDays = month > 2 && (year % 4 == 0 && (year % 100 != 0 || year % 400 == 0)) ? 1 : 0;
return DAYS_BEFORE[month - 1] + dayOfMonth + leapDays;
}This is all that is needed. 6 lines instead of 84. So when there should ever be a bug in the code, it will be easy to find, since there are only 3 lines where it could hide.
Code Snippets
private static final int[] DAYS_BEFORE = {0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334};
public static int dayOfYear(int month, int dayOfMonth, int year) {
int leapDays = month > 2 && (year % 4 == 0 && (year % 100 != 0 || year % 400 == 0)) ? 1 : 0;
return DAYS_BEFORE[month - 1] + dayOfMonth + leapDays;
}Context
StackExchange Code Review Q#143618, answer score: 10
Revisions (0)
No revisions yet.