Java class that does calculations between dates

Question

I am writing a simple java class for date calculations, I am aware there are probably better ones out there, but I want to do this myself for practicing purposes. I am planning on expanding this class but I am posting this here mainly because this is the first time I am doing something OOP related and the first time I am using java as well and I want some feedback.

package oop; public class Date { private String hour = null; private String minute = null; private String second = null; private String day = null; private String month = null; private String year = null; private String meridiem = null; public Date(String hour, String minute, String second, String meridiem) { this.hour = hour; this.minute = minute; this.second = second; this.meridiem = meridiem; } public Date(String day, String month, String year) { this.day = day; this.month = month; this.year = year; } public Date(String hour, String minute, String second, String meridiem, String day, String month, String year) { this.hour = hour; this.minute = minute; this.second = second; this.meridiem = meridiem; this.day = day; this.month = month; this.year = year; } public String concatenateDate() { String fullDate = ""; if(hour != null) { fullDate += hour + ":" + minute + ":" + second; } if(day != null) { if(hour != null) fullDate += " "; fullDate += day + "/" + month + "/" + year; } return fullDate; } public int daysUntilNextYear() { int nextYear = 0; int daysLeft = -1; System.out.println(year != null); if(year != null) { daysLeft = daysDistance(new Date("01", "01", Integer.toString(Integer.parseInt(year)+1))); } return daysLeft; } private int getDaysGone(int day, int month, int year) { int daysGone = day; if(month == 2) daysGone += 31; else if(month == 3) daysGone += 59; else if(month == 4) daysGone += 90; else if(month == 5) daysGone += 120; else if(month == 6) daysGone += 151; else if(month == 7)daysGone += 181; else if(month == 8)daysGone += 212; else if(month == 9)daysGone += 243; else if(month == 10) daysGone +=273; else if(month == 11)daysGone += 304; else if(month == 12)daysGone += 334; if(isLeapYear(year) && month > 2) { daysGone ++; } return daysGone; } private boolean isLeapYear(int year) { if(year%400 == 0 || (year%100 != 0 && year%4 == 0 )) { return true; } else { return false; } } public int daysDistance(Date secondDate) { int daysDistance = 0; try { int year = Integer.parseInt(this.year); int month = Integer.parseInt(this.month); int day = Integer.parseInt(this.day); int secondYear = Integer.parseInt(secondDate.getYear()); int secondMonth = Integer.parseInt(secondDate.getMonth()); int secondDay = Integer.parseInt(secondDate.getDay()); daysDistance += (secondYear - year) * 365; daysDistance += (getDaysGone(secondDay, secondMonth, secondYear) - getDaysGone(day, month, year) -1); daysDistance = Math.abs(daysDistance); if(secondYear > year) { for(int i = year+1; i < secondYear; i++) { if(isLeapYear(i)) { daysDistance ++; } } if(isLeapYear(year) && month < 3) daysDistance++; if(isLeapYear(secondYear) && secondMonth > 3) daysDistance++; } else if(secondYear == year) { if(secondMonth > month) { if(isLeapYear(year) && month < 3 && secondMonth > 3) daysDistance ++; } else { if(isLeapYear(year) && month > 3 && secondMonth < 3) daysDistance ++; } } else { for(int i = secondYear+1; i < year; i++) { if(isLeapYear(i)) { daysDistance ++; } } if(isLeapYear(year) && month > 3) daysDistance ++; if(isLeapYear(secondYear) && secondMonth < 3) daysDistance++; } } catch(Exception e) { daysDistance = -1; } return daysDistance; } public int secondsUntilDate(Date secondDate) { int secondsLeft = 0; try { secondsLeft += (daysDistance(secondDate)) * 86400; secondsLeft += 86400 - (Integer.parseInt(hour)*3600 + Integer.parseInt(second)) ; } catch(Exception e) { secondsLeft = -1; } return secondsLeft; } public String getDay() { return day; } public String getMonth() { return month; } public String getYear() { return year; } public String getHour() { return hour; } public String getMinute() { return minute; } public String getSecond() { return second; } public String getMeridiem() { return meridiem; } } 

Answer 1

1. why is your instance variables defined as String? it makes no sense. it allows user of your class to pass invalid values and it forces you to parse into int in several places. (and you should know that parseInt() may throw an exception if the value is not numeric). it makes much more sense to have the variables defined as int.

2. you should validate your input. months are between 1 and 12 and days should be validated according to the given month. you can either throw a custom checked exception (to force clients to catch it), or use the JDK unchecked IllegalArgumentException

3. constructor overloading: you have 3 constructors. one that is a combination of the other two. If you want to follow the DRY rule, then you probably want to make private methods like setTime(...) and setDate(...). then have each constructor call the approperiate method and the 7 arg constructor will call both. The methods is the place where you will want to perform validation as well.

4. 7 arg constructor, all args of the same type - that's going to be a problem for your users. they have no idea of the order of args (unless you provide extensive javadoc documentation, which you didn't). A good fit here is the Builder pattern that allows the construction of object that require many args and/or configuration. so your user can do:
   Date today = new Date().withYear(2018).withMonth(6).withDayOfMonth(8);

5. avoid magic numbers. use constants. instead of 86400 define private static final int SECONDS_IN_DAY (and assign it a multiplication of SECONDS_IN_HOUR) this makes the code more readable and you also support other planets in our solar system.

6. getDaysGone() - now wouldn't it be more useful to have a map of months and their respective number of days? this will help in validation as well

7. what about daylight saving?

Answer 2

I think the first thing you should do is defining the Date interface which lists all functionalities then implement it effectively by your way. You should not implement first and then think to born some useful functions. You can read Java Calendar to know more what they did and what they provided.

Answer 3

Note that it took years and years to get to the new Java date / time API. Stephen Colebourne wrote the Joda-Time API before heading the new API team. Time is an obnoxiously hard nut to crack. If this is an assignment then the date / time subject is probably chosen on purpose: it will generate an endless number of irregularities that can be useful for teaching.

There are several time-based standards, e.g. an ISO norm that specifies how UTC needs to behave and how to format canonical UTC based dates. If you would implement this for a company then you should:

1. lookup the standards and
2. find a matching implementation for keeping time.

Then you should not write wrapper classes around an existing API for generic operations: only create use-case specific classes and methods where required.

---

The Date class in Java itself uses milliseconds from 1970. This makes it easy to perform calculations, while you can still convert back to local time (using the badly named and implemented Calendar class).

This should also protect you from weird situations where the day (of month) is set, but the month isn't: you just choose the first day of month if it is not present.

In other words: it protects your class instances from ever reaching an invalid state. Avoiding invalid states is very important from a security and maintainability perspective. It is one of the reasons why immutable classes are often preferred as, internally, they can only have a single state (a point in time). These classes don't have setters, instead they require that you call methods to create new instances, e.g. nextDay(): Date.

One of the principles for defensive programming is called fail-fast where you immediately throw an exception instead of having to deal with invalid state. Validating the parameters of your constructors is an important first step to create a fail-fast implementation.

I would highly recommend to have a look at "Effective Java" from Joshua Bloch and then take a look at the new Java time API to see how it is implemented.

---

Code specific comments

Meridian (meridiem?) isn't really used in the class or in the calculations. As long as this is not the case, you should leave it out because your calculations should take it into account. Meridians aren't used in time calculations that use local time; the time zone and daylight saving may be.

---

Instead of constructors without specific names it is better to use factory methods, e.g. createDate(int year, int month, int dayOfMonth): Date. This makes it immediately clear what the arguments are.

---

Instead of returning -1 for daysUntilNextYear() you should make your class in such a way that it cannot return a bad value for the method.

Currently you are deferring the handling of the issue to the user of the class. That means that the user needs to implement a condition, throwing an Exception when -1 is returned. Even worse, if this is forgotten then the user will end up with a bad calculation result. That means that the error is propagated in a different, hard to catch form!

If you keep to the current design then you could either throw an Exception yourself or return an Optional<Integer> value. But these are both band-aid solutions.

---

if(year%400 == 0 || (year%100 != 0 && year%4 == 0 )) { return true; } else { return false; } 

Ah, my young Padawan, here you show your inexperience:

return year % 400 == 0 || (year % 100 != 0 && year % 4 == 0 ) 

also: do keep the spaces for readability. Can you see the difference?

---

if(month == 2) daysGone += 31; else if(month == 3) daysGone += 59; else if(month == 4) daysGone += 90; else if(month == 5) daysGone += 120; else if(month == 6) daysGone += 151; else if(month == 7)daysGone += 181; else if(month == 8)daysGone += 212; else if(month == 9)daysGone += 243; else if(month == 10) daysGone +=273; else if(month == 11)daysGone += 304; else if(month == 12)daysGone += 334; 

It isn't often the case but here a switch with fall-thru is probably a good idea.

switch (month) { case 12: daysGone += 31; // fall-thru (to next case) case 11: daysGone += 30: // fall-thru case 2: // do something special here for special years default: throw new RuntimeException("Unknown month: " + month); } 

using a constant such as public static final int DECEMBER = 12 would make your code more readable:

case DECEMBER: daysGone += 31; // fall-thru (to next case) 

It would also allow your users to reuse your constant. In modern Java you could use an enum with a method getCommonDays for the month (with Common to allow for February) but it may be a bit early to go into that.

getDaysGone should useisLeapYear for February than requiring that you adjust the date later.

---

It's unclear from the method what concatenateDate(): String does. You could create a method called readableDate() instead. You could also implement / override the toString() method for debugging.