CODEWITHSUNDEEP
java
Mike Sickler
Mike Sickler

Reputation: 34451

Grouping objects by date: am I an idiot?

I have a list of objects called Activity:

class Activity {
   public Date activityDate;
   public double amount;
}

I want to iterate through List, group them by date and return a new list . Here's what I currently do:

private List<Activity> groupToList(List<Activity> activityList) {
    SimpleDateFormatter sdf = new SimpleDateFormatter("YYYY-MM-DD");
    Map<String,Activity> groupMap = new HashMap<String,Activity>();
    for (Activity a in activityList) {
        String key = sdf.format(a.getActivityDate());
        Activity group = groupMap.get(key);
        if (group == null) {
          group = new Activity();
          groupMap.add(key, group);
        }
        group.setAmount(group.getAmount() + a.getAmount());
    }
    return new ArrayList<Activity>(groupMap.values()); 
}

Is it a WTF to use the DateFormatter in this way?

Upvotes: 4

Views: 3393

Answers (5)

Thomas Jung
Thomas Jung

Reputation: 33092

As already mentioned the best solution is to represent your date with day precission. If this is not possible joda is nice library.

If you can ignore daylight saving time then grouping by date can be accomplished much easier. A unix time day is 86 400 s long. The timestamp does ignore leap seconds. (Your timer stops for one second or the leap second is distributed in some way.) All date values were day is equal are the same day:

int msPerDay = 86400 * 1000;
long day = new Date().getTime() / msPerDay

One minor point is to adjust the timezone. For my timezone CET (UTC/GMT +1 hour) the GMT day starts one our later:

new GregorianCalendar(2009, 10, 1, 1, 0).getTime().getTime() / msPerDay) ==
new GregorianCalendar(2009, 10, 2, 0, 59).getTime().getTime() / msPerDay) ==
new Date().getTime() / msPerDay

If the daylight saving time is significant the best way is to use joda. The rules are just to complicated and locale specific to implement.

Upvotes: 0

Andreas Petersson
Andreas Petersson

Reputation: 16518

java.util.Date is a quite poor abstraction for your need; it is IMO fair to stick to strings if nothing better is around, HOWEVER Joda-time provides a good datatype for you: DateMidnight or alternatively LocalDate if Activity is strictly timezome-independant.

other than that, the code looks good to me, you might be able to shorten it a bit using an implementation of Multimap, to avoid messy null-checking code. to be honest, it doesn't get much shorter than your solution:

    public List<Activity> groupedByDate(List<Activity> input) {
    //group by day
    final Multimap<DateMidnight, Activity> activityByDay 
   = Multimaps.index(input, new Function<Activity, DateMidnight>() {
            @Override
            public DateMidnight apply(Activity from) {
                return new DateMidnight(from.activityDate);
            }
        });
    //for each day, sum up amount
    List<Activity> ret = Lists.newArrayList();
    for (DateMidnight day : activityByDay.keySet()) {
        Activity ins = new Activity();
        ins.activityDate = day.toDate();
        for (Activity activity : activityByDay.get(day)) {
            ins.amount+=activity.amount;
        }
    }
    return ret;
}

Upvotes: 1

Paul Tomblin
Paul Tomblin

Reputation: 182782

You could do this using the Date as the key if you used a TreeMap and provided a Comparator that only compared the year, month and day and not the time.

Upvotes: 0

Yishai
Yishai

Reputation: 91881

I would just use the date object itself as the key. If it it bothers you because the date object is mutable, then use its toString() value. No reason to go making formats.

If the issue is that you want to normalize the date by removing the time component, it would be much better to do that withing the Activity object and remove the time component. If the issue is still further that there are potential time zone issues, I would use JodaTime, but there is no object in the JDK currently that represents a pure date without time, so going with a string isn't outrageous, but it should be hidden behind a method in the Activity object and the fact that it is a date formatted string without a time component should be an implementation detail.

Upvotes: 2

Thomas
Thomas

Reputation: 181785

Why not simply create a HashMap<Date, Activity>() instead of the roundabout way with Strings?

Sorry, I didn't answer the question. The answer is: yes, unless I am an idiot ;)

Upvotes: 0

Related Questions