9

I looked through different questions here, but unfortunately I couldn't find an answer. This is my code:

SceneDelegate.swift

...
let contentView = ContentView(elementHolder: ElementHolder(elements: ["abc", "cde", "efg"]))
...
window.rootViewController = UIHostingController(rootView: contentView)

ContentView.swift

class ElementHolder: ObservableObject {

    @Published var elements: [String]

    init(elements: [String]) {
        self.elements = elements
    }
}

struct ContentView: View {

    @ObservedObject var elementHolder: ElementHolder

    var body: some View {
        VStack {

            ForEach(self.elementHolder.elements.indices, id: \.self) { index in
                SecondView(elementHolder: self.elementHolder, index: index)
            }

        }
    }
}

struct SecondView: View {

    @ObservedObject var elementHolder: ElementHolder
    var index: Int

    var body: some View {
        HStack {
            TextField("...", text: self.$elementHolder.elements[self.index])
            Button(action: {
                self.elementHolder.elements.remove(at: self.index)
            }) {
                Text("delete")
            }
        }
    }

}

When pressing on the delete button, the app is crashing with a Index out of bounds error.

There are two strange things, the app is running when

1) you remove the VStack and just put the ForEach into the body of the ContentView.swift or

2) you put the code of the SecondView directly to the ForEach

Just one thing: I really need to have the ObservableObject, this code is just a simplification of another code.

UPDATE

I updated my code and changed Text to a TextField, because I cannot pass just a string, I need a connection in both directions.

Lupurus
  • 3,618
  • 2
  • 29
  • 59

2 Answers2

8

The issue arises from the order in which updates are performed when clicking the delete button.

On button press, the following will happen:

  1. The elements property of the element holder is changed
  2. This sends a notification through the objectWillChange publisher that is part of the ElementHolder and that is declared by the ObservableObject protocol.
  3. The views, that are subscribed to this publisher receive a message and will update their content.
    1. The SecondView receives the notification and updates its view by executing the body getter.
    2. The ContentView receives the notification and updates its view by executing the body getter.

To have the code not crash, 3.1 would have to be executed after 3.2. Though it is (to my knowledge) not possible to control this order.

The most elegant solution would be to create an onDelete closure in the SecondView, which would be passed as an argument.

This would also solve the architectural anti-pattern that the element view has access to all elements, not only the one it is showing.

Integrating all of this would result in the following code:

struct ContentView: View {
    @ObservedObject var elementHolder: ElementHolder

    var body: some View {
        VStack {
            ForEach(self.elementHolder.elements.indices, id: \.self) { index in
                SecondView(
                    element: self.elementHolder.elements[index],
                    onDelete: {self.elementHolder.elements.remove(at: index)}
                )
            }
        }
    }
}

struct SecondView: View {
    var element: String
    var onDelete: () -> ()

    var body: some View {
        HStack {
            Text(element)
            Button(action: onDelete) {
                Text("delete")
            }
        }
    }
}

With this, it would even be possible to remove ElementHolder and just have a @State var elements: [String] variable.

Palle
  • 11,511
  • 2
  • 40
  • 61
  • 1
    Thanks, that makes it clear, why the app crashes: SecondView will be informed first (I tried to set breakpoints, but I couldn't figure this out). Unfortunately your solution is not what I'm searching for, because my `SecondView` needs much more other properties of the `ElementHolder`, so I can't pass just a String... that is why your code is working, because the SecondView doesn't hold the ElementHolder – Lupurus Apr 25 '20 at 19:33
  • 1
    ... and I have a `TextField`, not just a `Text`, please see my updated code – Lupurus Apr 25 '20 at 19:49
  • You could pass the item represented by each SecondView through a Binding. The binding works in two ways, so you pass the value from the ContentView to a SecondView and the SecondView passes changes back to the ContentView. Deletion would still have to be handled separately, as shown in my post. You can create a Binding using `Binding(get: {self.elementHolder.elements[index]}, set: { newValue in self.elementHolder.elements[index] = newValue})`. In the SecondView, there would have to be a variable `@Binding var value`. – Palle Apr 25 '20 at 23:40
  • @Lupurus Have you get any lucky? I have a same situation over here. A list contains elements and each of these elements shoudl have been binded through the views just to be able to update them in one view and publish those other views. It works like a charm without the `onDelete` – Faruk May 11 '20 at 19:51
  • @Palle It works as it is, however when I set the `SecondView` as a destionation to `NavigationLink` it does not work. Any ideas? – Faruk May 12 '20 at 02:12
  • 1
    @Faruk: It doesn't work, if you use it with `.indices`. Just don't use `.indices`, you need to iterate over the elements themselves and then you have to work with `.firstIndex(where: { $0.id == element.id })`, if you want the index of each element – Lupurus May 12 '20 at 08:18
  • @Lupurus OK, but how do you update the `element` inside the view with that way? – Faruk May 12 '20 at 14:56
  • @Faruk: With `Binding(get: { element }, set: { self.elementHolder.elements[self.elementHolder.elements.firstIndex(where: { $0.id == element.id} ?? 0] = $0 } )` – Lupurus May 12 '20 at 16:31
  • @Lupurus in the `SecondView`? – Faruk May 12 '20 at 19:37
4

Here is possible solution - make body of SecondView undependable of ObservableObject.

Tested with Xcode 11.4 / iOS 13.4 - no crash

struct SecondView: View {

    @ObservedObject var elementHolder: ElementHolder
    var index: Int
    let value: String

    init(elementHolder: ElementHolder, index: Int) {
        self.elementHolder = elementHolder
        self.index = index
        self.value = elementHolder.elements[index]
    }

    var body: some View {
        HStack {
            Text(value)     // not refreshed on delete
            Button(action: {
                self.elementHolder.elements.remove(at: self.index)
            }) {
                Text("delete")
            }
        }
    }
}

Another possible solution is do not observe ElementHolder in SecondView... for presenting and deleting it is not needed - also no crash

struct SecondView: View {

    var elementHolder: ElementHolder // just reference
    var index: Int

    var body: some View {
        HStack {
            Text(self.elementHolder.elements[self.index])
            Button(action: {
                self.elementHolder.elements.remove(at: self.index)
            }) {
                Text("delete")
            }
        }
    }
}

Update: variant of SecondView for text field (only changed is text field itself)

struct SecondViewA: View {

    var elementHolder: ElementHolder
    var index: Int

    var body: some View {
        HStack {
            TextField("", text: Binding(get: { self.elementHolder.elements[self.index] },
                set: { self.elementHolder.elements[self.index] = $0 } ))
            Button(action: {
                self.elementHolder.elements.remove(at: self.index)
            }) {
                Text("delete")
            }
        }
    }
}
Asperi
  • 228,894
  • 20
  • 464
  • 690