1
\$\begingroup\$

I am fairly new to the Groovy arena. I recently modified some code to add the following block. This code is added to remove dates we were supposed to remove from the total counts. This data goes on reports that a bug was noticed, that we weren't removing the queue entry dates.

result.processed.each{
    def queueEntry = QueueEntry.findById(it.id)<<<START ADD>>>
    Set dates = new HashSet<Long>()

    def children = QueueEntry.findAllByParent(queueEntry)

    for(QueueEntry qe : children){
        def f = new GregorianCalendar()
        f.setTimeInMillis(DateUtils.getClearedTime(qe.entryTimestamp))
        def l = new GregorianCalendar()
        l.setTimeInMillis(DateUtils.getClearedTime(qe.exitTimestamp))
        while(f < l){
            if(f.get(Calendar.DAY_OF_WEEK) != Calendar.SUNDAY && f.get(Calendar.DAY_OF_WEEK) != Calendar.SATURDAY){//only add weekdays
                dates.add(f.time.time)
            }
            def xx = new GregorianCalendar()
            xx.setTimeInMillis(f.time.next().time)
            f = xx
        }
        dates.add(l.time.time)
    } <<<STOP ADD>>>
    Set outsideDays = it.numberOfDaysOutsideCVB
    Set days = DateUtils.businessDaysBetweenDates(it.entryTimestamp, it.exitTimestamp)
    days.removeAll(outsideDays)
    days.removeAll(dates)
    turnTimes << days.size()
}

The application is now crawling. I am obviously doing something wrong. When this is run for small data-sets it will complete slowly. On larger sets it doesn't finish. Prior to this change it was completing.

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

I found the logic of the set operations a bit hard confusing. It appears that the goal is to take some set of days, named days, and remove entries from it. So, start with days and remove entries from it. You don't need the other sets named outsideDays and dates.

It's weird that it.numberOfDaysOutsideCVB returns a set rather than a number, as suggested by the method name.

The looping mechanism is cumbersome:

def xx = new GregorianCalendar()
xx.setTimeInMillis(f.time.next().time)
f = xx

The l day is always removed, regardless of the day of the week. That deserves an explanatory comment. Furthermore, f and l are somewhat cryptic variable names.

Set days = DateUtils.businessDaysBetweenDates(it.entryTimestamp, it.exitTimestamp)
days.removeAll(it.numberOfDaysOutsideCVB)

def children = QueueEntry.findAllByParent(QueueEntry.findById(it.id))
for (QueueEntry qe : children) {
    def entry = new GregorianCalendar()
    entry.setTimeInMillis(DateUtils.getClearedTime(qe.entryTimestamp))
    def exit = new GregorianCalendar()
    exit.setTimeInMillis(DateUtils.getClearedTime(qe.exitTimestamp))

    entry.upto(exit.previous()) { day ->
        switch (day.get(Calendar.DAY_OF_WEEK)) {
          case Calendar.SATURDAY:
          case Calendar.SUNDAY:
            break
          default:
            days.remove(day.time.time)
        }
    }
    // Remove the last day regardless of day of week. (Why?)
    days.remove(exit.time.time)
}
turnTimes << days.size()
\$\endgroup\$
  • \$\begingroup\$ I was not the original developer of this. This code that I added was pulled from another report. I believe the f is first and l is last. The last or l is removed to take off the last day in the calculation. Thanks for all your help and suggestions! \$\endgroup\$ – Zirous Tom Apr 9 '15 at 13:08
0
\$\begingroup\$

Here's a way to create a List of Lists, with the inner List containing Calendar instances which include only weekdays within the two timestamps.

BONUS: The code is concurrent so it can process multiple QueueEntries at the same time. That could help with the large data sets.

import static groovyx.gpars.GParsPool.withPool

class QueueEntry {
    Date entryTimestamp
    Date exitTimestamp
}

// Create some QueueEntr(ies)
def entries = [
    [entryTimestamp: new Date(), exitTimestamp: new Date() + 10],
    [entryTimestamp: new Date() + 5, exitTimestamp: new Date() + 12],
].collect { it as QueueEntry }

// Compute the dates concurrently.
withPool {
    entries.parallel
        .map { 
        // Convert each entry into a Groovy Range of Calendar(s)
        def start = it.entryTimestamp.clone().clearTime().toCalendar()
        def end = it.exitTimestamp.clone().clearTime().toCalendar()

        start..end
    }.map {range ->
        /*
         * Iterate through all the Calendars in the Range and collect
         * the ones that are weekdays.
         */
        range.findAll {
            !(it[Calendar.DAY_OF_WEEK] in [Calendar.SATURDAY, Calendar.SUNDAY])
        }
    }.reduce { a, b -> [a, b] } // Reduce to a List of List(s).
}.each {dates ->
    // Do as you please with the list of dates (Calendar instances).
    print dates.time 
}

I think the Calendar Range iteration may not be concurrent. I keep thinking that part can be fine-tuned.

\$\endgroup\$

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.