UPGRADE YOUR SKILLS: Learn advanced Swift and SwiftUI on Hacking with Swift+! >>

SOLVED: Review of my code

Forums > SwiftUI

Dear community,

I developed an app that calculates the expenses and credits of a user. I want to show this app to a potential employee for an internship.

I have some problems:

1 - In the tabview, when adding the first card, the plus button is inaccessible.

2 - The total amount of the card is calculated adding all the transactions. The user, when deleting a transaction, can choose if a transaction can remain in the card total amount or not. So when doing this the computed property recalculates the total showing a visual bug. Do you have better approaches for this?

3 - I need some revision on the MVVM architecture because I wasn't able to move all the properties to the vm file.

4 - General review of the code, is there some bad or "spaghetti" code? If so can you suggest some improvements?

The project is on github, it doesn't have a readme because I want to solve the first problem as fast as I can.

Link: [https://github.com/andreasara-dev/MoneyManager.git]

Thank you all for the review

2      

Sorry for the slow repsonse but there is a quite alot of code. I will try to explain a few of the pionts.

You need to document your code so it easier to understand (you can use option+click to get a summary or drill down to the file quickly)

3 - You have groups the files with Model, Views, ViewModels, however (personal) I group them by view eg Start, Card etc with ALL the files that are used in that file so when you working on a view then find them easily! PS could not see any properties that did not have to in the ViewModel

4 - Had a look at one AddCardView and there quite alot of modifiers so I suggest that use of ViewModifliers.

Add a file I called it TextFieldViewModifier

struct TextFieldViewModifier: ViewModifier {
    @Environment(\.colorScheme) var colorScheme

    func body(content: Content) -> some View {
        content
            .padding()
            .border(LinearGradient.gradient(for: colorScheme))
    }
}

Also add a file called ButtonViewModifier

struct ButtonViewModifier: ViewModifier {
    @Environment(\.colorScheme) var colorScheme

    func body(content: Content) -> some View {
        content
            .padding()
            .padding(.horizontal)
            .foregroundStyle(Color.primary)
            .overlay {
                RoundedRectangle(cornerRadius: 10)
                    .stroke(LinearGradient.gradient(for: colorScheme), lineWidth: 2)
            }
    }
}

Now add a file Extension-View this will make the call site cleaner (note the documentation)

extension View {
    /// A modifier that give a gradient color border to a `TextField`.
    ///
    /// When applied it will change the border to a linear gradient from bottomTrailing to topLeading
    /// of [.pink, .purple, .black] or [.pink, .purple, .white] depending on light/dark mode.
    var textFieldViewModifier: some View {
        self.modifier(TextFieldViewModifier())
    }

    /// A modifier that give a gradient color `RoundedRectangle` to a `Button`
    ///
    /// When applied it will change the border to a linear gradient from bottomTrailing to topLeading
    /// of [.pink, .purple, .black] or [.pink, .purple, .white] depending on light/dark mode.
    var buttonViewModifier: some View {
        self.modifier(ButtonViewModifier())
    }
}

Now you can use in AddCardView (Also used Label in toolbar button - give accessibility label)

struct AddCardView: View {
    @EnvironmentObject var dataController: DataController
    @StateObject private var viewModel = ViewModel()
//    @Environment(\.colorScheme) var colorScheme
    @Environment(\.dismiss) var dismiss

    var body: some View {
        NavigationView {
            VStack {
                SecureField("Enter card number", text: $viewModel.cardNumber)
                    .keyboardType(.numberPad)
                    .textFieldViewModifier
//                    .foregroundColor(colorScheme == .light ? .black : .white) // ** NOT NEEDED **
//                    .padding()
//                    .border(LinearGradient.gradient(for: colorScheme))

                TextField("Enter balance", text: $viewModel.balance)
                    .keyboardType(.decimalPad)
                    .textFieldViewModifier
//                    .foregroundColor(colorScheme == .light ? .black : .white) // ** NOT NEEDED **
//                    .padding()
//                    .border(LinearGradient.gradient(for: colorScheme))

                Button { // ** YOU MIGHT WANT TO DISABLE BUUTON UNTIL CARD DETAILS AND AMOUNT ARE FILLED **
                    viewModel.maskCardNumber(dataController: dataController)
                    dismiss()
                } label: {
                    Text("Submit")
                        .buttonViewModifier
//                        .padding()
//                        .padding(.horizontal) // .horizontal same as [.leading, .trailing]
//                        .foregroundStyle(Color.primary) // use this it changes between black/white
//                        .foregroundColor(colorScheme == .light ? .black : .white)
//                        .overlay {
//                            RoundedRectangle(cornerRadius: 10)
//                                .stroke(LinearGradient.gradient(for: colorScheme), lineWidth: 2)
//                        }
                }
                .padding(.top)
//                .accessibilityLabel("Press this button for insert you card informations") // ** NO NEED AS "submit" IS BETTER **
                .accessibilityHint("Added!") // better to "this will add the new card"
                if !viewModel.errorMessage.isEmpty {
                    Text(viewModel.errorMessage)
                        .foregroundColor(.red)
                        .padding(.vertical)
                }
            }
            .padding()
            .toolbar {
                Button {
                    dismiss()
                } label: {
                    Label("Close", systemImage: "xmark.circle") // change to xmark.circle not x.circle
                        .symbolRenderingMode(.multicolor)
//                    Image(systemName: "x.circle")
//                        .foregroundColor(.red)
                }
//                .accessibilityLabel("Return to previous screen") // ** NO NEED AS USE OF LABEL **
            }
            .navigationTitle("Add Card")
            .navigationBarTitleDisplayMode(.inline)
        }
    }
}

By deleting the commented out code it will read easier.

I also noticed that the TextFields in WelcomeView and AddCardView are different. I would change the TextFieldViewModifier to

struct TextFieldViewModifier: ViewModifier {
    @Environment(\.colorScheme) var colorScheme

    func body(content: Content) -> some View {
        content
            .textFieldStyle(.roundedBorder)
            .overlay {
                RoundedRectangle(cornerRadius: 5)
                    .stroke(LinearGradient.gradient(for: colorScheme), lineWidth: 1)
            }
    }
}

now you can add this

Group {
    TextField("Name", text: $viewModel.firstName)
    TextField("Last Name", text: $viewModel.lastName)
}
.textFieldViewModifier

You could do the same with the button.

Just one other thing if you change LinearGradient to this (note .primary instead of .black/.white)

static var borderGradient: LinearGradient {
    LinearGradient(colors: [.pink, .purple, .primary], startPoint: .bottomTrailing, endPoint: .topLeading)
}

then every you used .gradient(for colorScheme: ColorScheme) you can use LinearGradient.borderGradient and not use colorScheme

struct TextFieldViewModifier: ViewModifier {
    func body(content: Content) -> some View {
        content
            .textFieldStyle(.roundedBorder)
            .overlay {
                RoundedRectangle(cornerRadius: 5)
                    .stroke(LinearGradient.borderGradient, lineWidth: 1)
            }
    }
}

Sent Pull Request on github of changes if you want to use it

2      

@NigelGee Thank you!

It means a lot for me. You're right, without the amount of modifiers I wrote, the code is so much cleaner, readable and maintanable.

I accepted the pull request, again thanks for the help.

2      

Hacking with Swift is sponsored by RevenueCat.

SPONSORED Take the pain out of configuring and testing your paywalls. RevenueCat's Paywalls allow you to remotely configure your entire paywall view without any code changes or app updates.

Learn more here

Sponsor Hacking with Swift and reach the world's largest Swift community!

Reply to this topic…

You need to create an account or log in to reply.

All interactions here are governed by our code of conduct.

 
Unknown user

You are not logged in

Log in or create account
 

Link copied to your pasteboard.