1
\$\begingroup\$

I've finally got a solution for a "weekly streak" bucketing system I've been working on but unfortunately it's resulted in a very large switch statement.

I'm running through a handful of dates and determining by a range of seconds (the length of a week) which bucket that date belongs to.

It's obvious to me that this could be refactored in a programmatic way because of the amount of repetition but I'm not sure where to start. Dictionary? Some kind of sorting algorithm? Recursion?

open func weeklyStreakCount(weeklyGoal target: Int) -> Int {
    let endDate = Date()
    let startDate = endDate.startOfWeek!
    let startDateInterval = Double(startDate.timeIntervalSinceNow)

    var workoutsPerWeek = [Int: Int]()
    let userWorkouts: [UserWorkoutEntity] = self.userWorkouts(completed: true)

    var numberOfGoodBuckets = 0

    for i in 0...100 {
        workoutsPerWeek.updateValue(0, forKey: i)
    }

    // calculate the time from now to seconds in a week and round to the nearest hundreths to create a bucket for that week
    for userWorkout in userWorkouts {
        let workoutTimeInterval = Double((userWorkout.completionDate?.timeIntervalSinceNow)!)
        let rawBucket = (startDateInterval - workoutTimeInterval) / numberOfSecondsInAWeek

        let bucket = Int(rawBucket * 1000)
        let abbrNumberOfSecondsInAWeek = 604

        switch bucket {
        case 0 ... abbrNumberOfSecondsInAWeek:
            workoutsPerWeek[0]! += 1
        case abbrNumberOfSecondsInAWeek ... (abbrNumberOfSecondsInAWeek * 2):
            workoutsPerWeek[1]! += 1
        case abbrNumberOfSecondsInAWeek * 2 ... (abbrNumberOfSecondsInAWeek * 3):
            workoutsPerWeek[2]! += 1
        case abbrNumberOfSecondsInAWeek * 3 ... (abbrNumberOfSecondsInAWeek * 4):
            workoutsPerWeek[3]! += 1
        case abbrNumberOfSecondsInAWeek * 4 ... (abbrNumberOfSecondsInAWeek * 5):
            workoutsPerWeek[4]! += 1
        case abbrNumberOfSecondsInAWeek * 5 ... (abbrNumberOfSecondsInAWeek * 6):
            workoutsPerWeek[5]! += 1
        case abbrNumberOfSecondsInAWeek * 6 ... (abbrNumberOfSecondsInAWeek * 7):
            workoutsPerWeek[6]! += 1
        case abbrNumberOfSecondsInAWeek * 7 ... (abbrNumberOfSecondsInAWeek * 8):
            workoutsPerWeek[7]! += 1
        case abbrNumberOfSecondsInAWeek * 8 ... (abbrNumberOfSecondsInAWeek * 9):
            workoutsPerWeek[8]! += 1
        case abbrNumberOfSecondsInAWeek * 9 ... (abbrNumberOfSecondsInAWeek * 10):
            workoutsPerWeek[9]! += 1
        default:
            break
        }
    }

    // Run through each bucket and see how many times the user hit their goal
    for i in 0...10 {
        if(workoutsPerWeek[i]! > target) {
            numberOfGoodBuckets += 1
        } else {
            break
        }
    }
    return numberOfGoodBuckets
}
\$\endgroup\$
  • \$\begingroup\$ Can you add more details on what you are doing? Why 10 buckets if a week has 7 days? Where does the "magic number" 604 come from? How is the times array defined? How is numberOfSecondsInAWeek calculated? – A self-contained compiling example would be helpful. \$\endgroup\$ – Martin R Jul 24 '17 at 6:53
  • \$\begingroup\$ Sure thing, I was trying to just hone in on the switch statement but I've updated the code with actual function and variable names. 604 comes from having ~604,000 seconds in a week, but I am trimming it and the end dates down to the first three digits. I suppose this is now unnecessary but a previous iteration I had created could only handle this approach. \$\endgroup\$ – Rudest Buddhist Jul 24 '17 at 7:03
  • 1
    \$\begingroup\$ Apparently you seek in which multiple of abbrNumberOfSecondsInAWeek the value of bucket falls. If I'm right, then why don't you just divide bucket by abbrNumberOfSecondsInAWeek and truncate the result to the nearest lower integer? \$\endgroup\$ – CiaPan Jul 24 '17 at 7:39
0
\$\begingroup\$

First, I would replace the dictionary

var workoutsPerWeek = [Int: Int]()

by an array. A dictionary could be memory-saving if you have "few" keys in a "large range", e.g. 1, 20, 300, 4000, as a "sparse-array" emulation. But that is not the case here: The bucket numbers range from 0 to 9, so that an array is more appropriate, easier to initialize, and easier to access (no forced unwraps needed).

It is unclear in your code how many entries are needed: You initialize entries for 0...100, update entries for 0...9, and finally evaluate entries 0...10. If you want to collect the data for the last 10 weeks then it would be

var workoutsPerWeek = Array(repeating: 0, count: 10)

Now you can use the computed bucket number as an index into the array and replace the switch statement by

let rawBucket = Int((startDateInterval - workoutTimeInterval) / numberOfSecondsInAWeek)
if rawBucket >= 0 && rawBucket < workoutsPerWeek.count {
    workoutsPerWeek[rawBucket] += 1
}

The range check for the bucket number can also be done as

if workoutsPerWeek.indices.contains(rawBucket) { ... }

There are more things which can be improved: In

let workoutTimeInterval = Double((userWorkout.completionDate?.timeIntervalSinceNow)!)

the conversion to Double is not needed. And what if completionDate is nil? Your code would crash in that case.

But my main point of criticism is how the bucket is computed. A day does not necessarily have 24 hours. In regions with daylight saving time, a day can have 23 or 25 hours if the clocks are adjusted one hour back or forward. Applied to your code: A week does not necessarily have 604,800 seconds.

The proper way to compute calendar differences is to use the Calendar methods and DateComponents:

for userWorkout in userWorkouts {
    // Ensure that completionDate is set, otherwise ignore this entry
    guard let completionDate = userWorkout.completionDate else { continue }

    // Compute #of weeks between completionDate and startDate
    let weeksAgo = Calendar.current.dateComponents([.weekOfMonth], from: completionDate, to: startDate).weekOfMonth!

    // Update corresponding bucket
    if workoutsPerWeek.indices.contains(weeksAgo) {
        workoutsPerWeek[weeksAgo] += 1
    }
}

Finally, the calculation of the number of consecutive weeks where the goal has been reached can be simplified to

let numberOfGoodBuckets = workoutsPerWeek.prefix(while: { $0 >= target }).count
\$\endgroup\$
  • \$\begingroup\$ Wow, this is so concise. Thanks for the effort! \$\endgroup\$ – Rudest Buddhist Jul 24 '17 at 16:58

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.