SOLVED: Better more efficient approach

@SplintaWM HWS+

Feb '23

Hi All,

I have the following code which uses a switch to return a dictionary key/value array based on a switch condition set elsewhere, at the moment as you can see, I have to use multiple for loops to illiterate over the indices of the values I'm looking for and it seems to me to be really inefficient as the for loops are used multiple times. I'm sure there is a better and more efficient approach to this, but I can't see it and I do have the need to use the function multiple times one after the other. Could anyone help with a better more efficient solution please?

My code:

 func getStatsFor(for period: CountHistory, itemCount: [ItemCount]) -> (total: [String:Int], singleTotal: Int) {
        let startOfWeek = Date.thisWeek
        let calender = Calendar.current
        var total: [String:Int] = [:]
        var singleTotal: Int

        switch period {
        case .today:
            for i in itemCount.indices {
                if calender.isDate(itemCount[i].countDate, equalTo: Date.now, toGranularity: .day) {
                    total["today"] = (total["today"] ?? 0)+Int(itemCount[i].countValue)
                }
            }
            singleTotal = total["today"] ?? 0

        case .yesterday:
            for i in itemCount.indices {
                if calender.isDateInYesterday(itemCount[i].countDate) {
                    total["yesterday"] = (total["yesterday"] ?? 0)+Int(itemCount[i].countValue)
                }
            }
            singleTotal = total["yesterday"] ?? 0

        case .thisweek:
            var tempWeekTotal = 0

            for i in itemCount.indices {
                if itemCount[i].countDate > startOfWeek {
                    let day = dayFromDate(date: itemCount[i].countDate)
                    total[day] = (total[day] ?? 0)+Int(itemCount[i].countValue)
                    tempWeekTotal += total[day] ?? 0
                }
            }
            singleTotal = tempWeekTotal

        case .month:
            for i in itemCount.indices {
                if calender.isDate(itemCount[i].countDate, equalTo: Date.now, toGranularity: .year) {
                    let month = monthFromDate(date: itemCount[i].countDate)

                    total[month] = (total[month] ?? 0)+Int(itemCount[i].countValue)

                }
            }
            singleTotal = 0           

        case .total:
            for i in itemCount.indices {
                total["total"] = (total["total"] ?? 0)+Int(itemCount[i].countValue)
            }
            singleTotal = total["total"] ?? 0
        }

        return (total, singleTotal)

    }

2

@Greenamberred HWS+

Feb '23

I haven't used all your structures. You could consider this, and use the princples behind it which are .filter and .reduce, adapting to your needs.

enum Groups {
    case one
    case two
    case three
}

func calculate(group: Groups, items: [Int]) -> (total: [String: Int], singleTotal: Int) {

    var total: [String:Int] = [:]
    var singleTotal: Int = 0

    switch group {
    case .one:
        singleTotal = items.reduce(0, +)
    case .two:
        total["Two"] = total["Two", default: 0] + items.filter { number in
            number.isMultiple(of: 2)
        }.reduce(0, +)
    case .three:
        total["Three"] = total["Three", default: 0] + items.filter { number in
            number.isMultiple(of: 3)
        }.reduce(0, +)
    }

    return (total, singleTotal)
}

print(calculate(group: .one, items: [1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20]))
print(calculate(group: .two, items: [1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20]))
print(calculate(group: .three, items: [1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20]))

2

@SplintaWM HWS+

Feb '23

Many thanks @Greenamberred, I have considered and dabbled using reduce and filter in my project and I did get it to (mostly) work however the issue that concerns me the most is having to illiterate over all items every time the function is called be this using a filter or in indices and I'm wondering if there is a way of "caching" the initial fetch so that on the next call it can just update that "cached" version.

Many Thanks, Nigel

2

@roosterboy HWS+

Feb '23

I wouldn't worry about it unless you notice it impacting performance. In which case you can use Xcode's tools to see where the slowdowns are and fix things accordingly. Unless you are dealing with absolutely enormous numbers of items, you really shouldn't see any problems; the standard library collection and sequence methods are pretty well optimized.

2

@roosterboy HWS+

Feb '23

And just by way of illustration, here's a solution I worked up.

Obviously, I don't have your full code, so I had to guess and fill in some blanks when it came to your data structures. Shouldn't really matter, I don't think, as the principles are the same.

You first have to create a method on the CountHistory enum that takes in a Date and determines if that Date falls within the period, returning a Bool result. This will be used to filter our items in the getStats(for:from:) function below. I called this method contains(date:).

import Foundation

enum CountHistory: String, RawRepresentable {
    //we make this enum conform to RawRepresentable
    //  so that we can get a name to use as a Dictionary key
    case today, yesterday, thisWeek, month, total

    //given an instance of this enum, we can check whether
    //  a particular Date falls within the period
    func contains(date: Date) -> Bool {
        let cal = Calendar.current

        switch self {
        case .today:
            return cal.isDateInToday(date)
        case .yesterday:
            return cal.isDateInYesterday(date)
        case .thisWeek:
            return cal.isDate(date, equalTo: .now, toGranularity: .weekOfYear)
        case .month:
            return cal.isDate(date, equalTo: .now, toGranularity: .month)
        case .total:
            //we don't care what the date is, we want everything
            return true
        }
    }
}

//The simplest ItemCount struct I could come up with that would work with the code
struct ItemCount {
    let countDate: Date
    let countValue: Int
}

//Now the heart of the matter...
func getStats(for period: CountHistory, from itemCount: [ItemCount]) -> (total: [String:Int], singleTotal: Int) {
    //start out by getting rid of any items not in the given period
    let filteredItems = itemCount.filter { period.contains(date: $0.countDate) }

    //initialize an empty Dictionary to hold our results
    var total: [String:Int] = [:]

    //we have to special case .thisWeek, since we want Dictionary entries for
    //  each weekday rather than one entry for the whole period
    if case .thisWeek = period {
        //loop through all of our filteredItems
        filteredItems.forEach {
            //pull out the weekday name for the item
            let weekday = $0.countDate.formatted(.dateTime.weekday(.wide))
            //add the countValue for the item using the weekday name as key
            total[weekday, default: 0] += $0.countValue
        }
    } else {
        //reduce all the filteredItems into a single sum and store it in the
        //  Dictionary using the period name as the key
        total[period.rawValue, default: 0] = filteredItems.reduce(0) { result, current in
            result + current.countValue
        }
    }

    let singleTotal = total.values.reduce(0, +)

    return (total, singleTotal)
}

And let's do some testing...

//Date extension to allow us to generate a random Date within a given range
extension Date {
    static func random(from startDate: Date, to endDate: Date) -> Date {
        //we'll get an error if endDate is before startDate, so make
        //  sure they are in the proper order
        let date1: Date = min(startDate, endDate)
        let date2: Date = max(startDate, endDate)

        //grab the number of seconds since 1970 for each end of the range
        let startTime = date1.timeIntervalSince1970
        let endTime = date2.timeIntervalSince1970

        //and pick a random value in the range
        let randomDate = TimeInterval.random(in: startTime...endTime)
        return Date(timeIntervalSince1970: randomDate)
    }
}

//set up some test data
let cal = Calendar.current
//start 5 months ago
let startDate = cal.date(byAdding: .month, value: -5, to: .now)!
//end 5 months from now
let endDate = cal.date(byAdding: .month, value: 5, to: .now)!

//generate 100 ItemCount instances
// with a random countDate between our startDate and endDate set above,
// and a random countValue between 15 and 150
let items: [ItemCount] = (1...100).map { _ in
    ItemCount(countDate: Date.random(from: startDate, to: endDate),
              countValue: Int.random(in: 15...150))
}

//print(items)

//try each period
print(getStats(for: .total, from: items))
//(total: ["total": 8668], singleTotal: 8668)
print(getStats(for: .today, from: items))
//(total: ["today": 37], singleTotal: 37)
print(getStats(for: .yesterday, from: items))
//(total: ["yesterday": 103], singleTotal: 103)
print(getStats(for: .month, from: items))
//(total: ["month": 973], singleTotal: 973)
print(getStats(for: .thisWeek, from: items))
//(total: ["Saturday": 43, "Sunday": 149, "Tuesday": 103], singleTotal: 295)

2

@SplintaWM HWS+

Feb '23

Thank you @roosterboy! You've recreated my structure really well to be fair and that was the solution I was looking for... I had tried using filter and reduce and it worked (mostly) but the obvious bit I missed was to check for the weekday case first and go from there!

I also appreciate that the execution time is always going to vary with any loop you do over any number of items as it's all dictated by the length of the array and the big O notation of that array.

Thank you both @roosterboy and @Greenamberred you have both been of great help and to be honest you have pointed out the obvious to me which is what I was missing!

Thank you both again!

2

Archived topic