Creating date strings and setting dates

Question

I've created a Java class, Date, in which I basically create date strings and set dates. Is there anything I can do besides adding comments to improve/shorten my code?

I've tested all of my methods to see if they work correctly, and they all work as planned. I haven't done tests that force the IllegalArgumentsException to be thrown yet though.

I've tried changing the instantiated boolean variable shortDisplay have a default value of true, which has made my program work correctly as far as I know.

I'm assuming when you declare a boolean instance variable, you need to also declare what its default value should be, otherwise issues arise. If anyone could comment on why that is, that would be great.

Date.java

public class Date {
    private int year;
    private int month;
    private int day;
    private boolean shortDisplay = true;

    public Date() {
        setDate(2000, 01, 01);
    }

    public Date(int yy, int mm, int dd) {
        setDate(yy, mm, dd);
    }

    public Date(int yy, int mm) {
        setDate(yy, mm);
    }

    public Date(int yy) {
        setDate(yy);
    }

    public Date(Date otherDate) {
        setDate(otherDate);
    }

    public Date(String dateStr) {
        setDate(dateStr);
    }

    public void setDate(int yy, int mm, int dd) {
        setYear(yy);
        setMonth(mm);
        setDay(dd);
    }

    public void setDate(int yy, int mm) {
        setYear(yy);
        setMonth(mm);
        setDay(1);
    }

    public void setDate(int yy) {
        setYear(yy);
        setMonth(1);
        setDay(1);
    }

    public void setDate(Date otherDate) {
        int yy = getYear();
        int mm = getMonth();
        int dd = getDay();
        setYear(yy);
        setMonth(mm);
        setDay(dd);
    }

    public void setDate(String dateStr) {
        int length = dateStr.length();
        int indexCount = 0;
        if (length >= 0) { // basically checking to see if the date even exists
            if (dateStr.indexOf('/') >= 0) {
                int index = dateStr.indexOf('/');
                indexCount++;
                String dd = dateStr.substring(index+1, dateStr.length());
                if (dd.substring(1, dd.length()).lastIndexOf('/') >= 0 ) {
                    int index2 =  dd.lastIndexOf('/');
                    indexCount++;
                    String yy = dd.substring(index2+1, dd.length());
                    if (yy.lastIndexOf('/') >= 0) {
                        indexCount++;
                    }
                    String mm = dateStr.substring(0, index);
                    dd = dd.substring(0, index2);
                    yy = yy.substring(0, yy.length());
                    System.out.println(mm + "\n" + dd + "\n" + yy );
                    int y = Integer.parseInt(yy);
                    int d = Integer.parseInt(dd);
                    int m = Integer.parseInt(mm);
                    setYear(y);
                    setMonth(m);
                    setDay(d);
                }
            }
        }

    }

    public void setYear(int yy) {
        if (yy >= 1900) {
            year = yy;
        } else {
            throw new IllegalArgumentException("lol");
        }
    }

    public void setMonth(int mm) {
        if (mm >= 1 && mm <= 12) {
            month = mm;
        } else {
            throw new IllegalArgumentException("not cool");
        }
    }

    public void setDay(int dd) {
        if (dd >= 1 && dd <= 31) {
            day = dd;
        } else {
            throw new IllegalArgumentException("stupid");
        }
    }

    public int getYear() {
        return year;
    }

    public int getMonth() {
        return month;
    }

    public int getDay() {
        return day;
    }

    public void setShortDisplay() {
        shortDisplay = true;
    }

    public void setLongDisplay() {
        shortDisplay = false;
    }

    public boolean isShortDisplay() {
        if (shortDisplay) {
            return true;
        } else {
            return false;
        }
    }

    public void incrementDay() {
        day++;
        if (day > 31) {
            day = 1;
            incrementMonth();
        }
    }

    public void incrementMonth() {
        month++;
        if (month > 12) {
            month = 1;
            incrementYear();
        }
    }

    public void incrementYear() {
        year++;
    }

    public boolean equals(Object other) {
        Date that = (Date) other;
        if (this.day == that.day && this.month == that.month && this.year == that.year) {
            return true;
        } else {
            return false;
        }
    }

    public boolean before(Date otherDate) {
        Date that = otherDate; 
        if (this.year < that.year) return true;
        if (this.month < that.month) return true;
        if (this.day < that.day) return true;
        else return false;
    }

    public String toString() {
        boolean check = isShortDisplay();
        if (check) {
            if (month < 10 && day < 10) {
                return "0" + month + "/" + "0" + day + "/" + year;
            }
            if (month < 10 && day > 10) {
                return "0" + month + "/" + day + "/" + year;
            }
            if (month > 10 || day < 10) {
                return month + "/" + "0" + day + "/" + year;
            } else {
                return  month + "/" + day + "/" + year;
            }
        } else {
            return monthString(month) + " " + day + ", " + year;
        }
    }

    public static String monthString(int month) {
        if (month >= 1 && month <= 12) {
            String[] stringMonth = {"", "January", "February", "March", "April", "June", "July",
                "August", "September", "October", "November", "December"};
        return stringMonth[month];
        } else {
            throw new IllegalArgumentException();
        }
    }
}

DateTest.java

public class DateTest {

    public static void main(String[] args) {
        // TODO Auto-generated method stub
        Date d = new Date(2013, 4, 9);
        System.out.println(d);
        d.setLongDisplay();
        System.out.println(d);

        Date d1 = new Date();
        if (!d1.toString().equals("01/01/2000")) {
            System.out.println("Error 1");
        }

        d1.setDate("4/8/2013");
        if (!d1.toString().equals("04/08/2013")) {
            System.out.println("Error 2");
        }

        d1.setDay(28);
        for (int i = 0; i<5; i++) {
            d1.incrementDay();
        }

        System.out.println(d1);

        if (!d1.toString().equals("05/02/2013")) {
            System.out.println("Error 3");
        }
    }
}

Naming any class such that it conflicts with a class in java.util is probably a bad idea. java.sql.Date is bad enough. — 200_success♦, yesterday

h.j.k. · Answer 1 · 2014-04-16 05:34:41Z

Your class does not handle months with less than 31 days, in-real-life-speaking your unit test for adding five days to 28th April is already wrong
stringMonth can be left as a private static final array, so that you are not re-creating it every time monthString() is called
equals() implementation does not check for null/class type, and by right you should have a corresponding hashCode() implementation too
before() can probably be better implemented as compareTo() if you can consider letting your class implement Comparable
There are better ways of formatting the String representation in toString() and parsing a String date in setDate(String dateStr)
Some Javadocs will be helpful too, especially to indicate the format that you are using in toString() ("MM/DD/YY")
Consider a proper testing framework such as JUnit or TestNG?

janos · Answer 2 · 2014-04-16 08:23:15Z

Write proper unit tests, instead of "checking with your eyes" the output of System.out.println, for example using JUnit4:

public class DateTest {
    @Test
    public void testShortAndLongDisplay() {
        Date d = new Date(2013, 4, 9);
        Assert.assertEquals("04/09/2013", d.toString());
        d.setLongDisplay();
        Assert.assertEquals("April 9, 2013", d.toString());
    }

    @Test
    public void testSetDate() {
        Date d1 = new Date();
        Assert.assertEquals("01/01/2000", d1.toString());

        d1.setDate("4/8/2013");
        Assert.assertEquals("04/08/2013", d1.toString());
    }

    @Test
    public void testIncrementDay() {
        Date d1 = new Date(2013, 4, 28);
        Assert.assertEquals("04/28/2013", d1.toString());
        for (int i = 0; i < 5; i++) {
            d1.incrementDay();
        }
        Assert.assertEquals("05/02/2013", d1.toString());
    }
}

There is a big bug in the before method, as it fails this unit test (the second assert):

@Test
public void testBefore() {
    Date d1 = new Date(2013, 4, 28);
    Date d2 = new Date(2013, 5, 27);
    Assert.assertTrue(d1.before(d2));
    Assert.assertFalse(d2.before(d1));
}

Some problems in the setDate method:

indexCount is incremented but never used
You should not print things in code with System.out.println. You might want to use a Logger instead.

Making toString depend on the shortDisplay smells, to me. It would be better to create String formatDate(boolean shortDisplay) method for different display purposes instead of using toString.

If you don't plan to create sub-classes of this one, then it's better to access shortDisplay directly within the class instead of the isShortDisplay accessor. But if you do plan sub-classes then never mind.

This is a bit awkward:

public boolean isShortDisplay() {
    if (shortDisplay) {
        return true;
    } else {
        return false;
    }
}

Simplify it to:

public boolean isShortDisplay() {
    return shortDisplay;
}

As others pointed out, the equals method doesn't check for null and class type. Also, the if-else there can be simplified:

@Override
public boolean equals(Object obj) {
    if (obj instanceof Date) {
        Date that = (Date) obj;
        return this.day == that.day
                && this.month == that.month
                && this.year == that.year;
    }
    return false;
}

It's also nice to explicitly add the @Override annotation when you're overriding a superclass method, just in case somebody reads the code (or a diff) outside a nice IDE.

And when you implement an equals method, you should also implement a hashCode method to ensure consistency, how about:

@Override
public int hashCode() {
    return String.format("%02d/%02d/%04d", day, month, year).hashCode();
}

setDate(2000, 01, 01);

IntelliJ gives me a warning about this, because 01 is an octal integer... Very cosmetic, but maybe you can just drop those zeros anyway....

"because 01 is an octal integer" Good catch, this means 08 wouldn't work. ;) — h.j.k., yesterday
I had the idea of returning something like what your hashCode() does, but I didn't know how to properly make that work. I haven't learned about using something like String.format, so I tried using printf. Also the print statements were there because I was having issues with grabbing the right numbers from a given string date, and so I used them to see what was being created. Thanks for all the tips. — cbenn95, 16 hours ago

raistlinthewizard · Answer 3 · 2014-04-17 09:28:32Z

I don't know why you want to write your own Date class. In Java you can use java.util.Date, or java.sql.Date with java.text.SimpleDateFormat. If you need your own Date class, you can write for instance a wrapper class or write an Util class.

Java Date api is ugly. In java 8 there is a new more usable date api. In earlier versions I recommend Joda-Time 3rd party library.

Review of Your code:

Unit tests: There are tools such as JUnit or TestNG to test your code. This way you can write more reusable, simpler, easy-to-use, automated tests. The real unit tests help your work, for instance you can describe the the expected behaviour in a more proper way.
Your Date class:
- Constructors There are a lot of constructors with different number of parameters. You can "embed" them with the this(...) method. For example:
```
public Date() {
//  setDate(2000, 01, 01);
  this(2000, 1, 1);
}
```
- Setters
  - The setDate(...) methods can invoke each similar as I wrote in the constructor section.
  - The setDate(Date otherDate) method is not working now. It's setting the owned values now. Instead of you can use:
```
public void setDate(final Date otherDate) {
    year = otherDate.year;
    month = otherDate.month;
    day = otherDate.day;
}
```
  - The setDay(...), setYear(...), setMonth(...) methods are invoked in the class for example in the constructors or in other setters. Robert Cecil Martin: Clean Code says: One Level of Abstraction per Function in Chapter 3: functions. So you can separate the validating from the setters for example this way:
```
private static final int MINIMUM_YEAR = 1900;

public void setYear(final int year) {
    this.year = checkYear(year);
}

public int checkYear(final int year) {
    if (year >= MINIMUM_YEAR) {
        return year;
    }

    throw new IllegalArgumentException("The year must be greater than " + MINIMUM_YEAR);
}
```
    Here 1900 was a "magic number", you should name it for increase readability. You should name your variables what they exactly. yy is ok in a date format, but as a variable it's exactly a year. Don't mind if it's hiding a field, in Java you can access your fields through 'this.'. For example this.year.
  - The setDate(String dateStr) method is so unreadable and splitting string with indexOf is so hard. You can use String.split(...) for example to get the values.
    - The dateAsString name is more readable than dateStr.
    - The indexCount variable is not used so you can remove it.
    - As Joshua Bloch's Effective Java says: Whenever possible use final variables. This minimize the scope of local variables.
  - Your setDate(...) methods of your Date class has tricky name, because in proper way one setter sets one field. So the year, month, day field trio represents the date here. It's not a main problem, but decreasing the code's readability.
- monthString(int month) You should use a private static array or an enum class, to don't redefine the mounts on every invokation.

asked	yesterday
viewed	86 times
active	today

current community

your communities

more stack exchange communities

Creating date strings and setting dates

3 Answers

Review of Your code:

Your Answer

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

Hot Network Questions

current community

your communities

more stack exchange communities

Creating date strings and setting dates

3 Answers

Review of Your code:

Your Answer

Sign up or log in

Post as a guest

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

Related

Hot Network Questions