Aovib Aovib - 1 year ago 43
Swift Question

Check validity of element in an array of objects and then retrieve that object and its index?

I have a simple array of objects:

var cars: [Car] = [Car(ID: "010", name: "car1"), Car(ID: "230", name: "car2"), Car(ID: "350", name: "car3")]

The user will use a
to enter an ID.

I will then have to check the array to see if there is an object that has this exact ID, if such an object exists I need to retrieve the object and the index where it is located.

In addition I need to update a flag variable to know whether the user entered a correct or incorrect number. As I will use this to change other aspects of the view controller (such as lay-out).

I currently have this function that receives the ID as parameter and then returns a tuple for another function. The carObject and indexOfCar are global variables that have to be updated:

func checkIdMatch(IdInput: String) -> (Bool, Car?) {
var returnObject: Car?
var tuple = (false, returnObject)

for (index, car) in cars.enumerated() {
if car.carId == IdInput {
returnObject = car
tuple = (true, car)

carObject = car
indexOfCar = index
flag = 2 //correct ID
} else {
flag = 1 //wrong ID
return tuple

I tried to use a
statement after the for()-statement, but then I couldn't update the flags correctly. What am I doing wrong?
Any help is immensely appreciated!

Answer Source

There's a lot going on here, I'm going to try to address everything, one at a time:

  1. In your code, you never break your loop early when you find a match.
    • There are obvious performance implications
    • Has the (probably undesired) side effect that flag keeps getting set over and over again. Thus, flag will only be 2 if the correct ID is found last in the array (so that there's no next element for which it'll be set back to 1).
  2. Your flag is a raw integer. It should probably be an enumeration.
  3. Your cars array has an unnecessary type annotation : [Car] can (and should) be left to the compiler to infer.
  4. Your (Bool, Car?) tuple is superfluous. It's a clever idea in feneral: pairing a data value with a data value (such as a Car instance, in this case) with a Bool value that represents whether or not the data value is non-null. Apple liked this design so much, in fact, that this is exactly how Optional works! You just need to return Car?, which can either be a valid instance, or nil.
  5. Your returnObject variable is never used, and ironically, never returned.
  6. Your function called checkIdMatch(IdInput:) not only checks if an ID matches, but also removes it from the array, changes global state, and refreshes your table view. This doesn't sound like "checking" to me!
  7. Your function name, and its parameter use Id, whereas your Car uses ID. Inconsistent.
  8. Your function's parameter doesn't need the Input suffix. We know that parameters are inputs.
  9. carObject and indexOfCar probably shouldn't be globals, but I don't have enough context to be able to tell. In any case, wouldn't indexOfCar always be inaccurate? You're storing the index where the Car is, but then you immediately remove the car, so the index is now wrong.
  10. You use copious amounts of so called "side effects". This is when a function has an effect apart from just processing its parameters into some return value. This is unavoidable in many cases, but having lots of side effects make code really hard to work with, hard to maintain, and hard to extend. Wherever possible, try to minimize side effects and reliance on global state. Ideally, you should try to write "pure" functions. Those that have no effect beyond returning a result. They're much, much easier to work with.

Here's my first attempt:

struct Car {
    let id: String
    let name: String

enum Flag: Int { //TODO: Name me!
    case original
    case noMatch
    case match

var flag = Flag.noMatch;

var cars = [Car(id: "010", name: "car1"), Car(id: "230", name: "car2"), Car(id: "350", name: "car3")]

func removeCar(ID: String) -> Car? {
    guard let index = cars.index(where: {$ == id}) else {
        flag = .noMatch //TODO: side effects make kittens cry
        return nil

    flag = .match //TODO: side effects make kittens cry
    return cars.remove(at: index)

print("Removed:\r\n\(removeCar(id: "350"))\r\n\r\n")

You can try it online, here.