Sqlite.swift -- Selecting all rows and returning them in an array of tuples

Question

Link to SQLite wrapper

func queryAll() -> [(String, String, String)] {
 do {
 let sql = try DB?.prepare("SELECT city, zip , temp FROM weather")
 var arr : [(city : String, zip : String, temp : String)] = []
 for row in sql! {
 arr += [(city: row[0] as! String, zip: row[1] as! String, temp: row[2] as! String)] 
 }
 return arr; 
 }
 catch {
 print("\(error)")
 }
 return [("","","")]
}

Grabbing all the rows, and returning them as a tuple for later use. I've read the docs but I cannot think of a more efficient way to return all the rows to a TableView

override func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
 let cell : CustomTableViewController = self.tableView.dequeueReusableCell(withIdentifier: "Cell")! as! CustomTableViewController
 let DB : WeatherDataStorage = WeatherDataStorage()
 var arr = [(city : String, zip : String, temp : String)]()
 arr = DB.queryAll()
 cell.cityLabel.text = arr[i].city
 cell.zipLabel.text = arr[i].zip
 cell.temperatureLabel.text = arr[i].temp
 i += 1
 return cell
}

EDIT: GIST

• Martin R: Is that your working code? I wonder about let cell : CustomTableViewController because a cell cannot be a view controller.
• Joseph: Cell is a custom table view cell class
• Martin R: But then let cell : CustomTableViewController makes no sense. Please post your actual working code. – The definitions of DB and WeatherDataStorage would also be relevant.
• Ryan H.: Suggestion: It is not necessary to define the type of a variable/constant that can be inferred by both the compiler and someone reading the call site. It adds additional noise when reading your code. Instead of let cell: CustomTableViewController... you can declare it as let cell = self.tableView...
• Ryan H.: Also, it would be more appropriate to name DB as db as per Swift's API Design Guidelines. Or in this case, something like dataStore.

Answer 1

1. Don't use tuples like this. The purpose of tuples is to make simple, short lived collections of values. For a use case like this, it is preferable to use a struct or a class to contain your results. There are many disadvantages.

2. Don't return non-sensical default values as a way of error handling. "" isn't a city, and "" isn't a valid zip code, so why try to claim them as such? This is what optionals are for, use them.

3. Narrow your do block to contain only the function calls that throw.

4. Avoid force casting if you can. Handle your errors sensibly.

5. Currently, you manually create an empty array, and repeatedly add to it in a for loop. This is discouraged for several reasons:

6. arr must be mutable.

7. It's a lot of boiler-plate code.

8. You can incur a lot of reallocation overhead, because you didn't call reserveCapacity on your array.

Using map is preferable instead.

6. Improve your naming. sql is a result set, it's not an sql code snippet, nor is it an sql connection. Name it so.

7. Don't use the pattern of: array += [newElement]. This technique causes an unnecessary array allocation. Instead, just use array.append(newElement).

8. Change your DB variable to not be an Optional. If there is something like a connection issue that causes DB to be nil, this should be handled at the time that you attempt to open the connection. Leaving it as an Optional, and using optional chaining DB?.doSomething() is a terrible idea. It makes the nil case fail silently, leading to very annoying to catch bugs.

9. "weather" is an awkward name for a table. Try something like "WeatherReading"

Here's how I would write this code:

struct WeatherReading {
 let city: String
 let zipCode: String
 let temperature: String
}
func queryAllWeatherReadings() -> [WeatherReading]? {
 guard let queryResults = try? DB.prepare("SELECT city, zip, temperature FROM WeatherReading") else {
 debugPrint(error)
 return nil
 }
 let weatherReadings = queryResults.map { row in
 guard let city = row[0] as? String,
 let zipCode = row[1] as? String,
 let temperature = row[2] as? String else {
 fatalError("There was a type error in the received result set: \(row)")
 }
 return WeatherReading(city: city, zipCode: zipCode, temperature: temperature)
 }
 return weatherReadings
}