4

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?

  • I'm using the DateFormatter because each activityDate could have time information.
2
  • private function ? is this java? Nov 1, 2009 at 14:57
  • Whoops! Fixed that. I've been doing a lot of Actionscript lately.. Nov 1, 2009 at 14:59

5 Answers 5

2

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.

0
1

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;
}
1
  • Date is fine for his purposes, imho you succeeded in making a simple algorithm needlessly more complex.
    – rsp
    Nov 1, 2009 at 16:16
0

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 ;)

5
  • Because if you do that, you'll get a different key for every millisecond instead of one for every day. Nov 1, 2009 at 14:53
  • The thing being called activityDate, I assumed that no time information was included.
    – Thomas
    Nov 1, 2009 at 14:59
  • Java Dates have time information unless you explicitly remove it. Nov 1, 2009 at 15:03
  • Yeah, this activityDate is coming from the database, and does have time information Nov 1, 2009 at 15:53
  • In that case, I'm the idiot. :) But you could still do it in this way, if you set the other fields from the Date to zero before using it as a key.
    – Thomas
    Nov 1, 2009 at 15:54
0

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.

0

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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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