Formatting date depending on current date

Question

I have two screens where I show a message creation time and date. Initially, there was only one screen and I should always show time and show date differently depending on difference between today and the message creation day.

Here is my method:

private final static String TIME_TEMPLATE_TODAY = "HH:mm";

public static String getFormattedDate(Date msgDate, Date currentTime) {
        long daysDiff = Utils.getDaysDiff(currentTime, msgDate);

        String format;
        if (currentTime.getYear() != msgDate.getYear()) {
            format = "dd.MM.yyyy' at '" + TIME_TEMPLATE_TODAY;
        } else if (daysDiff == 0) {
            format = TIME_TEMPLATE_TODAY;
        } else if (daysDiff == 1) {
            format = "'yesterday at " + TIME_TEMPLATE_TODAY;
        } else if (daysDiff > 1) {
            format = "dd.MM' at '" + TIME_TEMPLATE_TODAY;
        } else {
            format = "dd.MM.yyyy";
            Timber.e("Unexpected time diff: %d", daysDiff);
        }

        return new SimpleDateFormat(format, Locale.CANADA).format(msgDate);
    }

And now I should add cases for the second screen: show time only if it is today, otherwise, show only date (or "yesterday") without time.

I came up with solution below:

public static String getFormattedDate(Date msgDate, Date currentTime, boolean alwaysShowTime) {
        long daysDiff = Utils.getDaysDiff(currentTime, msgDate);
        StringBuilder format = new StringBuilder();

        // Add date representation
        if (currentTime.getYear() != msgDate.getYear()) {
            format.append("dd.MM.yyyy'");
        } else if (daysDiff == 1) {
            format.append("'вчера'");
        } else if (daysDiff > 1) {
            format.append("dd.MM");
        } else {
            format.append("dd.MM.yyyy");
            Timber.e("Unexpected time diff: %d", daysDiff);
        }

        // Add time
        if (daysDiff == 0) {
            format.append(TIME_TEMPLATE_TODAY);
        } else if (alwaysShowTime) {
            format.append("' at '")
                    .append(TIME_TEMPLATE_TODAY);
        }

        return new SimpleDateFormat(format.toString(), Locale.CANADA).format(msgDate);
    }

Disadvantages that I see are:

1. I changed method contract (but it used in 1-2 places, so it is not a big deal in this case)

2. I need to always pass this third parameter to the method. I can solve both points creating another method with a previous contract and pass some default value for alwaysShowTime

3. If some third option appears, I would have to change the contract again, changing Boolean to enum, for example.

What problems or possible optimizations do you see here? Should I just create a new method and extract some common logic to a private method instead changing existing one?

Answer 1

It's neatly formatted and follows standard naming-conventions. I found it easy to follow the logic.

I'm not sure why you switched from a String to a StringBuilder. It doesn't buy you anything, and makes the code more verbose. (If you did it as an optimization, the effort is misplaced. These days, Java compilers turn simple String concatenations into StringBuilder appends behind the scenes. In any case, even if you were doing this trillions of times in a tight loop, the possible savings from StringBuilder would be negligible compared to the time to format dates and whatever I/O you're doing).

Rather than using a static method, you could create an interface (e.g. DateRenderer) and one or more implementations. This would be a bit more object-oriented and more future proof. You wouldn't have to add the alwaysShowTime flag (or whatever future formatting options might be needed) to the interface, but have that knowledge in a specific implementation.

What is the foreign character string when daysDiff == 1? If it's not a typo, it might be worth writing a comment to explain, for people who can't read that language.

I'm not sure that TIME_TEMPLATE_TODAY is the right name, nor whether it is necessary. You don't use a constant for the other date/time formats in the method.

Answer 2

You only need daysDiff if it is false that currentTime.getYear() != msgDate.getYear(), so I would put everything with daysDiff in a code block in the else. You should only define variables where you need them.

If it is today, you trigger the else of the first if-clause:

   } else {
        format.append("dd.MM.yyyy");
        Timber.e("Unexpected time diff: %d", daysDiff);
    }

but I guess this is a mistake.

I have not tried it, but instead of a flag alwaysShowTime it might be cleaner to write two separate methods. They might share a third method that does some common work. It looks confusing as it is, and as I wrote above it seems there is a bug too.

Answer 3

I wonder why on New Years Day, you consider New Years Eve not to be yesterday.

I you can change the requirements, why not build something like this:

if (future) "dd.mm.yyyy at " + time;   // just in case
if (hoursDiff < 12) time;
if (daysDiff == 1) "yesterday at " + time;
if (yearsDiff == 0) "dd.mm.";
else "dd.mm.yyyy";

Answer 4

I think you can rewrite your code in such manner:

  public static String getFormattedDate(Date msgDate, Date currentTime, Format format) {
    return new SimpleDateFormat(format.get(msgDate, currentTime), Locale.CANADA).format(msgDate);
  }

  public enum Format {

    MandatoryTimeFormat {
      private static final String TIME_FORMAT = "HH:mm";

      @Override
      protected String get(int yearDiff, long daysDiff) {
        return daysDiff == 0
            ? BaseFormat.get(yearDiff, 0L) + TIME_FORMAT
            : BaseFormat.get(yearDiff, daysDiff) + "' at '" + TIME_FORMAT;
      }
    },

    BaseFormat {
      @Override
      protected String get(int yearDiff, long daysDiff) {
        if (yearDiff != 0) {
          return "dd.MM.yyyy'";
        }
        if (daysDiff == 1) {
          return "'yesterday'";
        }
        if (daysDiff > 1) {
          return "dd.MM";
        }
        return "dd.MM.yyyy";
      }
    };

    public String get(Date date1, Date date2) {
      return get(date2.getYear() - date1.getYear(), Utils.getDaysDiff(currentTime, msgDate));
    }

    protected abstract String get(int yearDiff, long daysDiff);
  }

Also, in my opinion it is better to use java.time.LocalDateTime and java.time.OffsetDateTime to work with time.