4
\$\begingroup\$

I have implemented the Caesar Cipher in Swift and incorporated it into an iOS-app.

I guess it's formally correct. Please see the attached images, which show how the usage of the app is meant.

Sim 01 Sim 02 Sim 03 Sim 04

Here's the relevant code:

struct ContentView: View {
    @State var input = ""
    @State var output = ""
    @State var shift  = 1
    @State var hasSubmitted = false
    
    var body: some View {
        VStack {
            Text("Caesar Cipher").font(.title)
            Form {
                Section("Text to Encrypt/Decrypt") {
                    TextField("Text to Encrypt/Decrypt", text: $input)
                        .lineLimit(2)
                        .disabled(hasSubmitted)
                        .textInputAutocapitalization(.never)
                }
                
                Section("Shift") {
                    Picker("Digits", selection: $shift) {
                        ForEach(1...25, id: \.self) { digit in
                            Text("\(digit)")
                        }
                    }
                }
                
                Section("Submit") {
                    HStack {
                        if hasSubmitted {
                            Spacer()
                            Button("Reset") {
                                input = ""
                                output = ""
                                hasSubmitted = false
                                shift = 1
                            }.buttonStyle(.borderedProminent)
                            Spacer()
                        } else {
                            Spacer()
                            ChoiceButton(caption: "Encrypt",
                                         input: $input,
                                         output: $output,
                                         hasSubmitted: $hasSubmitted) { index, count in
                                (index + shift) % count
                            }
                            Spacer()
                            ChoiceButton(caption: "Decrypt",
                                         input: $input,
                                         output: $output,
                                         hasSubmitted: $hasSubmitted) { index, count in
                                ((index - shift) + count) % count
                            }
                            Spacer()
                        }
                    }
                }
                
                Section("Encrypt/Decrypt Result") {
                    Text(output).bold()
                }
            }
        }
        .padding()
    }
}


struct ChoiceButton: View {
    var caption: String
    let alphabet = ["a", "b", "c", "d", "e", "f",
                    "g", "h", "i", "j", "k", "l",
                    "m", "n", "o", "p", "q", "r",
                    "s", "t", "u", "v", "w", "x",
                    "y", "z"]
    @Binding var input: String
    @Binding var output: String
    @Binding var hasSubmitted: Bool
    var algo: (Int, Int) -> Int
    
    var body: some View {
        Button(caption) {
            guard input.isEmpty == false else {
                return
            }
            
            let lInput = input.lowercased()
            hasSubmitted = true
            output = ""
            
            for char in lInput {
                let index = alphabet.firstIndex(of: String(char))
                
                if let index = index {
                    let shiftedIndex = algo(index, alphabet.count)
                    output = "\(output)\(alphabet[shiftedIndex])"
                } else {
                    output = "\(output)\(char)"
                }
            }
        }.buttonStyle(.bordered)
    }
}

Any feedback according the naming, form, algorithm, closure-usage, state-handling, app-implementation in general is welcomed.

Looking forward to reading your comments and answers.

\$\endgroup\$

1 Answer 1

6
\$\begingroup\$

My main point of criticism is that the encryption logic is in the view, actually in two views: the ContentView contains the code to shift one index forward or backward, and the ChoiceButton contains the code to apply that to strings. This should be factored out into a separate function

func caesar(input: String, shift: Int) -> String {
    // ...
}

which makes things much clearer and simpler. It also allows to add unit test for the encryption function.

Even better, this refactoring makes the ChoiceButton obsolete: we can replace

ChoiceButton(caption: "Encrypt",
             input: $input,
             output: $output,
             hasSubmitted: $hasSubmitted) { index, count in
    (index + shift) % count
}
Spacer()
ChoiceButton(caption: "Decrypt",
             input: $input,
             output: $output,
             hasSubmitted: $hasSubmitted) { index, count in
    ((index - shift) + count) % count
}

by

Button("Encrypt") {
    hasSubmitted = true
    output = caesar(input: input, shift: shift)
}
.buttonStyle(.bordered)
.disabled(input.isEmpty)
Spacer()
Button("Decrypt") {
    hasSubmitted = true
    output = caesar(input: input, shift: -shift)
}
.buttonStyle(.bordered)
.disabled(input.isEmpty)

The .disabled(input.isEmpty) replaces your test for an empty input field. In addition, it makes it visible to the user whether the buttons are active or not.

The encryption code itself can also be simplified a bit. If we declare alphabet as an array of Character then alphabet.firstIndex() can be called without casting the argument to a string. The encryption operation is a map: each character is mapped to a new character, therefore I would write it as such.

The function then looks like this:

func caesar(input: String, shift: Int) -> String {
    let alphabet: [Character] = ["a", "b", "c", "d", "e", "f",
                                 "g", "h", "i", "j", "k", "l",
                                 "m", "n", "o", "p", "q", "r",
                                 "s", "t", "u", "v", "w", "x",
                                 "y", "z"]
    let count = alphabet.count
    
    return String(input.lowercased().map { char in
        if let index = alphabet.firstIndex(of: char) {
            let shiftedIndex = (index + shift + count) % count
            return alphabet[shiftedIndex]
        } else {
            return char
        }
    })
}

Some more thoughts:

There is repeated information in the user interface, e.g. "Text to Encrypt/Decrypt" both as section header and as placeholder string. I would use only one or the other. Also a section header "Submit" above the Encrypt and Decrypt button is not necessary.

It looks better if the form is embedded in a NavigationView with a .navigationTitle, instead of a VStack:

var body: some View {
    NavigationView {
        Form {
            // ...
        }
        .navigationTitle("Caesar Cipher")
    }
}

enter image description here

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.