2

I am having an issue with my code not displaying the correct or desired output. Here is my code that I wrote up.

import java.time.LocalDateTime;
public class DriversLicense
{
   private String name;
   private int id;
   private int expYear;
   private int expMonth; 
   private int expDay;


    public DriversLicense(String name, int id, int expYear, int expMonth, int expDay)
    {
        this.name = name;
        this.id = id;
        this.expYear = expYear;
        this.expMonth = expMonth;
        this.expDay = expDay;
    }

    public boolean isExpired()
    {
        LocalDateTime date = LocalDateTime.now();

        boolean tORf = false;

        int year = date.getYear();
        int month = date.getMonthValue();
        int day = date.getDayOfMonth();

        if(year > this.expYear && month > this.expMonth && day > this.expDay)
        {
            return true;
        }
        return tORf;
    }

    public void displayInfo()
    {
        System.out.println("Name: " + this.name);
        System.out.println("ID: " + this.id);
        System.out.println("Expiration: " + this.expYear + "/" + this.expMonth + "/" + this.expDay);
    }
}

In my isExpired() method it is supposed to check weather the current date is later than the expiration date of an ID. And if the ID is expired it should print out true, else it should print out false. For some reason I am getting all false when it should be false true false for the three ids my program checks. Here is my test file below.

public class TestDL
{
   public static void main(String[] args)
   {
      DriversLicense dr1 = new DriversLicense("John Smith", 192891, 6, 21, 2018);
      dr1.displayInfo();
      System.out.println("Expired? " + dr1.isExpired());
      System.out.println();


      DriversLicense dr2 = new DriversLicense("Jennifer Brown", 728828, 5, 31, 2017);
      dr2.displayInfo();
      System.out.println("Expired? " + dr2.isExpired());
      System.out.println();

      DriversLicense dr3 = new DriversLicense("Britney Wilson", 592031, 7, 15, 2019);
      dr3.displayInfo();
      System.out.println("Expired? " + dr3.isExpired());
      System.out.println();
   }
}

Also here is the output I am currently getting.

Name: John Smith
ID: 192891
Expiration: 6/21/2018
Expired? false

Name: Jennifer Brown
ID: 728828
Expiration: 5/31/2017
Expired? false

Name: Britney Wilson
ID: 592031
Expiration: 7/15/2019
Expired? false
Ole V.V.
  • 81,772
  • 15
  • 137
  • 161
pro5476
  • 33
  • 1
  • 6
  • 3
    There are already methods in `LocalDate` and `LocalDateTime` that do this for you. Why do you feel the need to reinvent the wheel here? – Joe C Jan 29 '18 at 06:46
  • Also, please [take the tour](http://stackoverflow.com/tour) to see how the site works and what questions are on topic here, and [edit] your question accordingly. See also: [How to Debug Small Programs](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/) – Joe C Jan 29 '18 at 06:46
  • Take a long hard look at your logical condition. – Robby Cornelissen Jan 29 '18 at 06:50
  • you'll need several nested if statements in your isExpired method – Stultuske Jan 29 '18 at 06:52
  • 1
    Reading your `if(year > this.expYear && month > this.expMonth && day > this.expDay)` line I see, that your license is only expired, when the year, month and day (all three of them) are later than the `LocalDateTime.now();` and the parttimes But a license can expire not only when it is a year, a month and a day over the date, but also timespans that are shorter than one year, a month and a day. So your `isExpired` should get more checks than this one. As suggested by others, you should use the methods `LocalDate` and `LocalDateTime` provide for checking if a date is later than an other date. – MenuItem42 Jan 29 '18 at 07:02

2 Answers2

5

You can simply use LocalDate API for your DriversLicense class as shown below:

public class DriversLicense {

   private String name;

   private int id;

   private LocalDate expiryDate;

   public DriversLicense(String name,int id,int expYear,int expMonth,int expDay) {
      this.name = name;
      this.id = id;
      this.expiryDate = LocalDate.of(expYear, expMonth, expDay);
   }

   public boolean isExpired() {
      LocalDate currentDate = LocalDate.now();
      return currentDate.isAfter(expiryDate);
   }

   public void displayInfo() {
      System.out.println("Name: " + this.name);
      System.out.println("ID: " + this.id);
      System.out.println("Expiration: " + 
         expiryDate.getYear() + "/" + expiryDate.getMonth() + "/" + 
          expiryDate.getDayOfMonth());
   }

   @Override
   public String toString() {
      return "name=" + name + ", id=" + id + ", expiryDate=" + expiryDate;
   }
}

As a side note, just remember that you can use toString() as shown above if you wanted to look at the values inside the object.

Also, remove your System.out.println() with some logging frameworks like log4j api.

Vasu
  • 21,832
  • 11
  • 51
  • 67
  • Wouldn't it be expired if `expiryDate` is in the past, i.e. `expiryDate` is before today, aka today is after `expiryDate`, aka `currentDate.isAfter(expiryDate)`? – Andreas Jan 29 '18 at 07:07
  • Good and correct answer. Now we’re at using the standard classes more, we may also use a `DateTimeFormatter` for formatting the date rather than concatenating the string ourselves. – Ole V.V. Jan 29 '18 at 09:29
1

When you compare multiple "nested" values like the 3 date fields, you should only compare month values when the year values are equal, because the month comparison doesn't make sense when the years are different.

To illustrate when you compare which fields, here is a table:

+==================================================+================+
|                 Tests to perform                 |   Conclusion   |
+================+=================================+================+
| year1 < year2  |                                 |                |
+----------------+------------------+              |                |
|                | month1 < month2  |              | Date1 < Date2  |
|                +------------------+--------------+                |
|                |                  | day1 < day2  |                |
|                |                  +--------------+----------------+
| year1 == year2 | month1 == month2 | day1 == day2 | Date1 == Date2 |
|                |                  +--------------+----------------+
|                |                  | day1 > day2  |                |
|                +------------------+--------------+                |
|                | month1 > month2  |              | Date1 > Date2  |
+----------------+------------------+              |                |
| year1 > year2  |                                 |                |
+----------------+---------------------------------+----------------+

So as you can see, your test is woefully incomplete:

year > this.expYear && month > this.expMonth && day > this.expDay

I'll leave it up to you to code the correct test, though combining the 3 expXxx fields into a LocalDate object in the constructor would be far easier, since LocalDate objects know how to do this comparison.

Andreas
  • 154,647
  • 11
  • 152
  • 247