1
\$\begingroup\$

I work with an API where I need to write code to generate n appointment dates in the coming m months (from today). For the purpose, I write a Random date generator class provided. One condition is I need to use the java.sql.Date for the class.

import java.sql.Date;
import java.time.LocalDate;
import java.util.Calendar;
import java.util.GregorianCalendar;
import java.util.Objects;
import java.util.Random;
public class RandomDate {


    private final Random random;

    private final Date currentDate;
    private final int months;

    public RandomDate(Random random, Date currentDate, int months) {

        this.random = random;
        this.currentDate = currentDate;
        this.months = months;
    }

    public Date getRangeEndDate() {

    Calendar calendar = new GregorianCalendar();

    calendar.setTimeInMillis(this.currentDate.getTime());
    calendar.add(Calendar.DATE, this.months * 30);

    return new Date(calendar.getTimeInMillis());
}

public Date generateRandomDate(Date endDate) {

    int start = (int) this.currentDate.toLocalDate().toEpochDay();
    int end = (int) endDate.toLocalDate().toEpochDay();

    long randomDay = start + random.nextInt(end - start);
    return Date.valueOf(LocalDate.ofEpochDay(randomDay));
}

}

I have an issue that within the code I use the time classes from 3 different packages (java.sql, java.time, and java.util). How can I write it more elegantly?

\$\endgroup\$
  • \$\begingroup\$ Why would three different packages be a problem? Especially when one of them is java.util? \$\endgroup\$ – VLAZ Feb 10 at 15:24
  • \$\begingroup\$ I updated the code with only using the java.time.LocalDate \$\endgroup\$ – Arefe Feb 10 at 15:39
  • 1
    \$\begingroup\$ Why do you need to use java.sql.Date? This requirement may be 20 years old. Current code uses java.time.LocalDate instead. \$\endgroup\$ – Roland Illig Apr 12 at 4:20
3
\$\begingroup\$

The essential part of your code is:

static LocalDate random(LocalDate start, int months, Random rnd) {
    LocalDate end = start.plusMonths(months);
    int days = (int) ChronoUnit.DAYS.between(start, end);
    return start.plusDays(rnd.nextInt(days + 1));
}

I have no idea why you need more code than this to express the idea.

I don't see any practical reason to use either the old and ugly java.util.Date or the even uglier java.util.Calendar or the inappropriate java.sql.Date (since this code has nothing to do with databases, let alone SQL). All you need are the classes from java.time, and java.time.temporal.

\$\endgroup\$
-1
\$\begingroup\$

I updated the code with only using the java.time.LocalDate

public class RandomDate {

    private final LocalDate today;

    private final Random random;
    private final int months;


    public RandomDate(Random random, int months) {

        this.today = LocalDate.now();

        this.random = random;
        this.months = months;
    }


    public LocalDate getRangeEndDate() {

        LocalDate rangeEndDay = this.today.plusDays(this.months * 30);
        return rangeEndDay;
    }

    public LocalDate generateRandomDate(LocalDate endDate) {

        int start = (int) this.today.toEpochDay();
        int end = (int) endDate.toEpochDay();

        long randomDay = start + random.nextInt(end - start);
        return LocalDate.ofEpochDay(randomDay);
    }


    public LocalDate getToday() {
        return today;
    }

    public Random getRandom() {
        return random;
    }

    public int getMonths() {
        return months;
    }


    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (!(o instanceof RandomDate)) return false;
        RandomDate that = (RandomDate) o;
        return getMonths() == that.getMonths() &&
                Objects.equals(getToday(), that.getToday()) &&
                Objects.equals(getRandom(), that.getRandom());
    }

    @Override
    public int hashCode() {

        return Objects.hash(getToday(), getRandom(), getMonths());
    }

    @Override
    public String toString() {
        return "RandomDate{" +
                "today=" + today +
                ", random=" + random +
                ", months=" + months +
                '}';
    }

}
\$\endgroup\$
  • 1
    \$\begingroup\$ This code is overly long and hard-codes a month to be exactly 30 days, which is wrong. It also creates a class RandomDate for no apparent reason when a utility function would serve the same purpose. \$\endgroup\$ – Roland Illig Apr 12 at 3:57

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.