SwiftUI/Swift code for changing item values in an immutable struct

Question

just wanted someone to review my code and perhaps simplify it a bit. Everything is working as expected for the moment but it seems too much code for what i'm trying to achieve. Especially the function "toggleItems". It just feels ... not right.

GOAL: The idea is that i'm having a list with a fixed set of items. When selecting an item, an indicator needs to light up which indicates which list item was pressed last (active state). All other list items need to be deactivated at that point. At first i was using variables in my data model (struct) but wanted to avoid that they could be changed from without the struct so i tried using an immutable struct here (credits to "Swiftful Thinking" on Youtube)

SoundModel:

struct SoundModel: Identifiable {
 let id: String
 let name: String
 let displayName: String
 let isActive: Bool
 
 init(id: String = UUID().uuidString, name: String, displayName: String, isActive: Bool) {
 self.id = id
 self.name = name
 self.displayName = displayName
 self.isActive = isActive
 }
 
 func updateActiveStateToFalse() -> SoundModel {
 return SoundModel(id: id, name: name, displayName: displayName, isActive: false)
 }
 
 func updateActiveStateToTrue() -> SoundModel {
 return SoundModel(id: id, name: name, displayName: displayName, isActive: true)
 }
}

the SoundViewViewModel:

class SoundViewViewModel: ObservableObject {
 
 @Published var sounds: [SoundModel] = []
 
 init() {
 sounds.append(contentsOf: [
 SoundModel(name: "alarm1", displayName: "Alarm 1", isActive: true),
 SoundModel(name: "alarm2", displayName: "Alarm 2", isActive: false),
 SoundModel(name: "alarm3", displayName: "Alarm 3", isActive: false),
 SoundModel(name: "bird1", displayName: "Bird", isActive: false),
 SoundModel(name: "carhorn1", displayName: "Carhorn", isActive: false),
 SoundModel(name: "fireAlarm1", displayName: "Fire Alarm", isActive: false),
 SoundModel(name: "mellow", displayName: "Mellow", isActive: false),
 SoundModel(name: "meltdown1", displayName: "Meltdown", isActive: false),
 SoundModel(name: "schoolbel1", displayName: "Schoolbel", isActive: false),
 SoundModel(name: "siren1", displayName: "Siren", isActive: false),
 SoundModel(name: "ufo1", displayName: "Ufo", isActive: false),
 ]
 )
 }
 func toggleItems(soundItem: SoundModel) {
 if let index = sounds.firstIndex(where: { 0ドル.id == soundItem.id } ) {
 if !soundItem.isActive {
 sounds[index] = soundItem.updateActiveStateToTrue()
 
 }
 }
 
 for sound in sounds {
 if let index = sounds.firstIndex(where: { 0ドル.id == sound.id && 0ドル.id != soundItem.id }) {
 if sounds[index].isActive {
 sounds[index] = sound.updateActiveStateToFalse()
 }
 }
 }
 }
}

And my SoundView:

struct SoundsView: View {
 
 @EnvironmentObject var theme: ThemeManager
 
 //var soundManager = SoundManager()
 @StateObject var soundViewVM = SoundViewViewModel()
 
 var body: some View {
 List {
 ForEach(soundViewVM.sounds) { sound in
 Button {
 //soundManager.playSound(name: sound.name)
 soundViewVM.toggleItems(soundItem: sound)
 } label: {
 HStack(alignment: .center, spacing: 2) {
 Text(sound.displayName)
 Spacer()
 sound.isActive ? Circle().frame(width: 10).foregroundColor(theme.lightColor) : nil
 }
 }
 }
 }
 .listStyle(.elliptical)
 .navigationTitle {
 HStack {
 Text("Sounds")
 Spacer()
 }
 }
 }
}

• Bjorn Morrhaye: Hmm weird, let me review it for a sec. About the listStyle and navigationTitle, I'm using it in a watchOS app.
• Bjorn Morrhaye: It works here. I guess the issue is within the toggleItems(soundItem: SoundModel) function as I had the same behavior as well when I started, but like it's written here it should function correctly.
• Bjorn Morrhaye: You are right, I'm sorry. don't know how I copied that code wrong :/ sounds[index] = sound.updateActiveStateToFalse() does the trick (I already have the sound object from the for loop). I will edit again in my code. Now, looking at that function, should it be the right way to do it ? getting the indexes. Or is there a more simple manner ?

Answer 1

The toggleItems() function can be simplified to

func toggleItems(soundItem: SoundModel) {
 
 for (index, sound) in sounds.enumerated() {
 if sound.id == soundItem.id {
 sounds[index] = sound.updateActiveStateToTrue()
 } else {
 sounds[index] = sound.updateActiveStateToFalse()
 }
 }
}

with one simple loop instead of two loops in your code (where the second one is essentially a nested loop).

But what I would really do is to store the information about the currently selected sound in the view model, instead of having an isActive property in SoundModel:

class SoundViewViewModel: ObservableObject {
 
 @Published var sounds: [SoundModel]
 @Published var activeSoundId : SoundModel.ID?
 
 init() {
 let allSounds = [ ... ]
 sounds = allSounds
 activeSoundId = allSounds.first?.id
 }
}

Then making a sound "active" becomes as simple as

soundViewVM.activeSoundId = sound.id

without the need to traverse the array and updating each entry with a new (active or inactive) one. This makes the toggleItems() method obsolete, and the view code becomes

struct SoundsView: View {
 
 @StateObject var soundViewVM = SoundViewViewModel()
 
 var body: some View {
 List {
 ForEach(soundViewVM.sounds) { sound in
 Button {
 soundViewVM.activeSoundId = sound.id
 } label: {
 HStack(alignment: .center, spacing: 2) {
 Text(sound.displayName)
 Spacer()
 if sound.id == soundViewVM.activeSoundId {
 Circle().frame(width: 10)
 }
 }
 }
 }
 }
 }
}

The isActive property and the updateActiveStateToFalse/True() methods in SoundModel are no longer needed. Instead of converting the UUIDs to strings one can use the UUID directly for identification:

struct SoundModel: Identifiable {
 let id: UUID
 let name: String
 let displayName: String
 
 init(id: UUID = UUID(), name: String, displayName: String, isActive: Bool) {
 self.id = id
 self.name = name
 self.displayName = displayName
 }
}

• Bjorn Morrhaye: Wow very nice approach. Going to look into it in depth an try to use this approach more.