Working on a sorted by date function, need code review

This is related to iOS programing but its mostly swift code so I hope its ok here. The other places to ask these questions get far fewer to none in replies.

Since Swift does not have an Ordered Dictionary and I don't yet want to use an external library im working through variations of manipulating data arrays.

In this example I am presorting an array into multi-dimensional arrays to allow UITableView to read them as multi sections

While I wrote this a few days ago, and I hard tested it and it works, I come back to it today and im like wut!? I know what it needs to do, but I don't like how it reads and know I could do it cleaner, which is why I am here.

It needs to take an array of records, for each record wrap it into a new struct and generate a key using the openingDate for the record, add this new struct to a dictionary for uniqueness and sorting

Then take that dictionary and sort it by those named keys as dates it into an array of tuples.

Then move those tuples into a array as a mock Ordered dictionary for latter use

Thank you for any guidance

struct MockRecordDictionaryRow {
    let key : String
    let records : [DataRecordWrapped]
}

struct DataRecordWrapped {
    var key : String // in the format of Mon 3098
    let dateObject : Date
    let dateStringOriginal : String
    let dataRecord : DataRecord
    
    init(dataRecord: DataRecord, dateStringOriginal : String){
        self.key = ""
        self.dataRecord = dataRecord
        self.dateStringOriginal = dateStringOriginal
        //self.dateObject = Date()
        
        let dateFormat = "yyyy-MM-dd" // we atleast know the format from the json!!!
        let dateFormatterGet = DateFormatter()
        dateFormatterGet.dateFormat = dateFormat
        
        let gettingDate = dateFormatterGet.date(from: dateStringOriginal)
        if gettingDate != nil{
            self.dateObject = gettingDate!
            // auto build in the key
            dateFormatterGet.dateFormat = "MMM y"
            self.key = dateFormatterGet.string(from: gettingDate!)
        }
        else {
            self.dateObject = Date() // ?!??!?!! like when???
        }
        
        
    }
}


// Main sorting function

    func sortAndSplitRecords(_ records : [DataRecord]) -> [MockRecordDictionaryRow] {

                
        // prep a dictionary from the dates
        var recordByDateDictionary : [String : [DataRecordWrapped] ] = [:]
        // we won't sort the pending, the'll just be attached on later
        // these did not have opening dates
        var pendingEvents : [DataRecordWrapped] = []
        
        for item in records {
            let ww = DataRecordWrapped(dataRecord: item, dateStringOriginal: item.openeningDate?.openingDate ?? "")
            if ww.key.isEmpty == false {
                let key = ww.key
                if recordByDateDictionary.keys.contains(key) == false {
                    recordByDateDictionary[key] = []
                }
                recordByDateDictionary[key]?.append(ww)
                //else if ww.key.isEmpty == true {}
            }
            // we dont know how to handle the events that have no opening date yet
            else if ww.key.isEmpty == true{
                pendingEvents.append(ww)
            }
        }
        
        // we need to sort the dictioanry
        // sort the dictionary into a tuples array
        let datefind22 = DateFormatter()
        datefind22.dateFormat = "MMM-yy"
        
        // sort records by date into new array, why?
        var sorting3_dictionaryToTuples = recordByDateDictionary.sorted {
            let first = datefind22.date(from: $0.key)
            let second = datefind22.date(from: $1.key)
            return first! > second!
        }
        
        // at this point we have an array of tuples
        // which looks to be (key:String, value:[DataRecordWrapped])
        // its not really the struct we were after
        // could process it once more but thats changing the same and the type
        // but thats better still, so hmmmmm looking for a solution like maybe casting
        // look into https://github.com/apple/swift-collections/blob/main/Documentation/OrderedDictionary.md
        
        // for now lets hitch the pendingEvents into the bottom of the stack
        if pendingEvents.count > 0 {
            let unknowns = (key:"pending", value:pendingEvents)
            sorting3_dictionaryToTuples.append(unknowns)
        }
        
        // now what??
        
        // can convert to [MockRecordDictionary]
        var list : [MockRecordDictionaryRow] = []
        for item in sorting3_dictionaryToTuples {
            let yy = MockRecordDictionaryRow(key: item.key, records: item.value)
            list.append(yy)
        }
        
        
        return list
        
        
    }

I don’t have anything to contribute to your main question (sorry!) but I wanted to point out that your use of DateFormatter is likely to yield weird problems in the field. If you’re parsing a fixed-format date, you need to use the en_US_POSIX locale. QA1480 NSDateFormatter and Internet Dates has a detailed explanation as to why.

Share and Enjoy

Quinn “The Eskimo!” @ DTS @ Apple

Swift has an ordered dictionary in Collections. Don’t think of it as an external library: it’s more of an extension of the standard library.


As for making the code better, I highly recommend watching this:
https://developer.apple.com/wwdc18/223

In general, anything that can be implemented with method composition (lazily if the result is intermediate or otherwise not used repeatedly) should be. If an operation can’t be expressed in that fashion, and isn’t already in a core library like Algorithms, implement it yourself. This does wonders for readability, testability, and performance.


Make sure you use the appropriate methods of unwrapping Optional instances:

  • guard-let: Continue with non-optional value if it exists, exit scope otherwise
  • Optional.map(_:): Do something with value if it exists, propagate nil otherwise (I use this extremely often)
  • Postfix ?: Same as above, but specifically for accessing members of the value
  • nil-coalescing: Use value if it exists, or a different value if it doesn’t (use this whenever you have a reasonable “backup” in mind)
  • if-let: Do something with a value if it exists, potentially do something else if it doesn’t, continue afterwards either way (due to the other options, I use this less than you may expect)
  • switch: Do something based on the optional value (preferable if you want to do different things based on the actual value in addition to handling nil)
  • Optional.unsafelyUnwrapped: Use this if it is literally impossible for the value to be nil (for instance, because you just assigned a value to it). As the name implies, the compiler won’t save you if you misuse this.
  • Postfix !: Use this if it is possible for the value to be nil, but you don’t want to handle that scenario. This should be extremely rare in practice. Using it often may be a sign you allowed an optional somewhere you shouldn’t have. Think of it like a function stub for handling nil that hasn’t been implemented yet.

Finally, avoid repeatedly initializing DateFormatter instances: they’re expensive. It’s usually better to make them (private!) static/global variables. If you want an example, here’s my approach to reading the Date header of HTTPURLResponse instances.

I appreciate it! This will save me some headaches later

Thank you! ill watch the video now and research the info you sent. Im currently refactoring the whole function and some other sections. I knew a bit about the date being a slow down which is why I was doing this longer drawn out caching mechanism but it starts to make the logic get busy I du know.
This function only runs when needed so its I think pretty performant thus far but I still don't like the read of it.

As for the extension ill look into it, as long as it's solid and uptodate I can use it, im just weary about so many externals like with Node.js projects. But thats my opinion only

Swift Algorithms and Swift Collections, among others, are literally made by the Standard Library team: I wouldn't treat them as external dependencies. They’re certainly more reliable than Foundation, which has a litany of issues with cross-platform support and legacy behavior, powerful as it may be.

I quickly glanced through this code without going into too much details.

This sequence:

if recordByDateDictionary.keys.contains(key) == false {
	recordByDateDictionary[key] = []
}
recordByDateDictionary[key]?.append(ww)

can be expressed as:

recordByDateDictionary[key, default: []].append(ww)

Also a minor style comment: I personally never compare non optional values to false or true (but that's of course a question of marmite):

if ww.key.isEmpty == false {
--->
if !ww.key.isEmpty {

if ww.key.isEmpty == true {
--->
if ww.key.isEmpty {

Here:

    var list : [MockRecordDictionaryRow] = []
    for item in sorting3_dictionaryToTuples {
        let yy = MockRecordDictionaryRow(key: item.key, records: item.value)
        list.append(yy)
    }

you can use map instead:

    let list = sorting3_dictionaryToTuples.map { item in
        MockRecordDictionaryRow(key: item.key, records: item.value)
    }

Overall, I'd probably redo it in a more functional style, it feels too procedural for me.

1 Like

Exactly: raw loops can rapidly become relatively difficult to follow and optimize, and they often lead to issues like unnecessary allocation. When I want one, I usually move it outside the point of use and into a protocol extension, which allows it to be tested and optimized separately.

If the parameters line up exactly with the function you are calling, you can even skip the closure and express that directly:

let list = sorting3_dictionaryToTuples.map(MockRecordDictionaryRow.init)
1 Like