5

Is it considered bad practice to do this

I am checking if the array is nil, then force unwrapping to retrieve an element from the array. I am doing this to avoid creating an unnecessary if let variable.

if self.arrayOfStrings != nil{
    textLabel.text = self.arrayOfStrings![0]
}
Brian Green
  • 215
  • 3
  • 14
  • Other than it not being as readable, it should be fine. – LinusGeffarth Jul 30 '18 at 17:26
  • The `if let` variable is not unnecessary. You will use it inside the `if`. That's the whole point. – rmaddy Jul 30 '18 at 17:26
  • He is saying that creating that variable is costing him memory. So by force unwrapping, he is saving that much memory. – Rakesha Shastri Jul 30 '18 at 17:27
  • I think, by *unnecessary*, OP means extra memory allocation that can be avoided in this case @rmaddy. – LinusGeffarth Jul 30 '18 at 17:27
  • 7
    `textLabel.text = arrayOfStrings?.first` – Leo Dabus Jul 30 '18 at 17:27
  • 2
    @LeoDabus even though this is not an answer to the question, it's the best solution in this case. – LinusGeffarth Jul 30 '18 at 17:28
  • 2
    How about declaring the array as non-optional? in most cases an empty array for *no data* is sufficient – vadian Jul 30 '18 at 17:30
  • @RakeshaShastri The OP does not say that (at least explicitly) and there wouldn't be any extra memory use anyway. – rmaddy Jul 30 '18 at 17:31
  • And don't forget to check if the array indices contains the index before trying to subscript your array – Leo Dabus Jul 30 '18 at 17:31
  • Also why is the editor unable to detect i am checking if self.arrayOfStrings is nil or not, but knows that an if let variable has been checked already. In other words, why is it forcing me to use '!' . – Brian Green Jul 30 '18 at 17:33
  • Strongly related: https://stackoverflow.com/q/29717210/1187415, https://stackoverflow.com/q/48858725/1187415 – Martin R Jul 30 '18 at 17:34
  • @BrainGreen What makes you think that optional binding takes more memory? – Alexander Jul 30 '18 at 17:53
  • @Alexander I guess my question is twofold, i wanted to avoid creating a new variable for readability, and i did assume a new variable would require the allocation of more memory. – Brian Green Jul 30 '18 at 18:02
  • @BrianGreen The fact that you checked for `!= nil` on one line does not in any way change the fact that on the next line it is still optional and could still be `nil`. – rmaddy Jul 30 '18 at 18:02
  • @BrianGreen And please [edit] your question with any clarifications. Posting comments is not a good way to post clarifications. – rmaddy Jul 30 '18 at 18:05
  • @rmaddy I see, the if let variable would not be an optional, but the arrayOfStrings is still a declared optional type. – Brian Green Jul 30 '18 at 18:14
  • @BrianGreen You can conditionally bind an optional to a non-optional local variable with the same name, that shadows the optional. `if let arrayOfStrings = self.arrayOfStrings { ...` – Alexander Jul 30 '18 at 18:18

2 Answers2

4

Is it considered bad practice to do this

if self.arrayOfStrings != nil{
    textLabel.text = self.arrayOfStrings![0]
}

Not bad practice, and I do sometimes do it; but it is unnecessary and a bit unusual, because this is the situation that conditional bindings are intended to resolve.

I am checking if the array is nil, then force unwrapping to retrieve an element from the array. I am doing this to avoid creating an unnecessary if let variable.

"Creating an if let variable" is not "unnecessary". It is the elegant way to handle this. What you are proposing to do is what is "unnecessary" (and it is wasteful because you are compelling the runtime to access self.arrayOfStrings twice).

You have two choices, depending on whether everything needs to come to a halt if arrayOfStrings is nil.

You can put a guard:

guard let arr = self.arrayOfStrings else {return}
textLabel.text = arr[0]

Or you can use a conditional binding:

if let arr = self.arrayOfStrings {
    textLabel.text = arr[0]
}

The difference (aside from the early exit) is the scope of arr.

So, although to some extent this is a matter of opinion, I would say that the problem here lies in your sense of what is elegant.

matt
  • 515,959
  • 87
  • 875
  • 1,141
4

Yes, it is bad practice. Perhaps not in this case specifically but it is possible that some other thread could update the property and make it nil between this thread checking for nil and then force-unwrapping.

Just do:

if let arrayOfStrings = self.arrayOfStrings {
    textLabel.text = arrayOfStrings[0]
}

There is no unnecessary variable here since you actually use the variable inside the if let.

Of course in this very specific case of trying to get the first value of an optional array, you can simply do:

textLabel.text = self.arrayOfStrings?.first

If you actually wanted something other than index 0, you should check the index:

if let arrayOfStrings = self.arrayOfStrings, someIndex < arrayOfStrings.count {
    textLabel.text = arrayOfStrings[someIndex]
}

In none of these cases is the "extra" variable a waste of effort or memory. It is being used in a read-only capacity as a constant and Swift is smart enough not to create a complete copy of the array in the process.

rmaddy
  • 314,917
  • 42
  • 532
  • 579