2
\$\begingroup\$

I wrote a class to return the number of whole weeks between months in the same year. I need suggestions on my code.

public int year;
public String m1;
public String m2;


public Solution(int Y, String A, String B) throws ParseException {
    this.year = Y;
    this.m1 = A;
    this.m2 = B;


    String dateOneInput = year + "-" + m1;
    String dateTwoInput = year + "-" + m2;

    SimpleDateFormat formatter = new SimpleDateFormat("yyyy-MMM");

    //DATE 1
    Calendar c1 = Calendar.getInstance();
    c1.setTime(formatter.parse(dateOneInput));
    //Get first monday of the month
    c1.set(Calendar.DAY_OF_WEEK, Calendar.MONDAY);
    c1.set(Calendar.DAY_OF_WEEK_IN_MONTH, 1);

    System.out.println("Date 1 : " + c1.getTime());

    //DATE 2
    Calendar c2 = Calendar.getInstance();
    c2.setTime(formatter.parse(dateTwoInput));
    //Get last sunday of the month
    c2.set(Calendar.DAY_OF_WEEK, Calendar.SUNDAY);
    c2.set(Calendar.DAY_OF_WEEK_IN_MONTH, -1);

    System.out.println("Date 2 : " + c2.getTime());

    //Difference in weeks
    int weeks = c2.get(Calendar.WEEK_OF_YEAR) - c1.get(Calendar.WEEK_OF_YEAR);
    System.out.println("Number of weeks :" + (1 + weeks));

}

/**
 * @param args the command line arguments
 * @throws java.text.ParseException
 */
public static void main(String[] args) throws ParseException {
    Solution s = new Solution(2012, "June", "July");
}
\$\endgroup\$
  • \$\begingroup\$ Welcome to CR! Are you on Java 8? \$\endgroup\$ – h.j.k. Jun 25 '15 at 11:46
1
\$\begingroup\$

If possible, use the new Java 8 datetime API (and there does not seem to be a constraint to use any older version). It is less cumbersome to do such operations there. You can get the difference in weeks easier and it is probably more readable. With TemporalAdjusters you can start from the first day of a month and go the the first monday.

    LocalDate firstMondayOfMonth = LocalDate.of(2015, 6, 1).with(TemporalAdjusters.nextOrSame(DayOfWeek.MONDAY));

    LocalDate lastDayOfMonth  = LocalDate.of(2015,6,1).with(TemporalAdjusters.lastDayOfMonth());
    LocalDate lastSundayOfMonth = lastDayOfMonth.with(TemporalAdjusters.previousOrSame(DayOfWeek.SUNDAY));

    long weeksBetweenDates = ChronoUnit.WEEKS.between(firstMondayOfMonth, lastSundayOfMonth.plusDays(1));

Apart from that, the class Solution does not do very much. You could simply use a method where you do your calculation. Your variables are not speaking. Don't use A,B etc. but startDate or something like that

EDIT: In Java 8, you can get the first day of the Week with WeekFields.of(Locale.getDefault()).getFirstDayOfWeek()

also, one method / class should only do one thing. For example, parsing your dates and calculating the difference between two dates are two different things and should be in two methods / classes. see https://en.wikipedia.org/wiki/Single_responsibility_principle

\$\endgroup\$
2
\$\begingroup\$
public int year;
public String m1;
public String m2;


public Solution(int Y, String A, String B) throws ParseException {
    this.year = Y;
    this.m1 = A;
    this.m2 = B;

These fields shouldn't be public. By making them public you've exposed them to the outside world. These are internal details that should be properly encapsulated. Truthfully though, you don't need them at all. I would just work directly with the arguments that you passed in. Rename them though. Your current names don't tell the client what kind of values to expect. What's an "A"? How would another dev know what to pass into this method without looking at the internals? They wouldn't.

public Solution(int year, String startMonth, String endMonth) throws ParseException
\$\endgroup\$
1
\$\begingroup\$

The week does not start with a Monday in all regions (e.g. in the U.S., Sunday is the first day of the week).

Therefore, instead of Calendar.MONDAY, you should use the Calendar method getFirstDayOfWeek() to determine the first day of the week (and compute the last day of the week from this value).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.