caesar cipher decoder and encoder - go

Question

I implemented my first algorithm in golang - the caesar cipher. Is there something i could do more efficiently? I am quite new to go and any improvement suggestions are welcome.

package main

import "fmt"

// +1 encoding, -1 decoding

func main() {
        var text string
        var choice int
        var shift int

        fmt.Print("Text: ")
        fmt.Scanf("%s", &text)

        fmt.Print("+1 encoding, -1 decoding: ")
        fmt.Scanf("%d", &choice)

        fmt.Print("Shift: ")
        fmt.Scanf("%d", &shift)

        fmt.Println(cipher(text, choice, shift))
}

func cipher(text string, choice int, shift int) string{
        chars := []rune(text)
        var result string
        for i := 0; i < len(chars); i++ {
                if chars[i] >= 'a' && chars[i] <= 'z' || chars[i] >= 'A' && chars[i] <= 'Z' {
                        dchar := chars[i] + rune(shift*choice)
                        if dchar >= 'a' && dchar <= 'z' || dchar >= 'A' && dchar <= 'Z' {
                                result += string(dchar)
                        } else {
                                result += string(dchar + rune(-26 * choice))
                        }
                } else {
                        result += string(chars[i])
                }
        }
        return result
}

peterSO · Accepted Answer · 2022-07-27 13:08:40Z

Your API does not appear to be a thoughtful design:

func cipher(text string, choice int, shift int) string

A good API design follows the shape of the problem, not the implementation. The problem:

The general shape of the Caesar cipher is an encryption inverse function.

For the Caesar cipher, an encryption package API,

package caesar

func Encrypt(plain string, key int) string

func Decrypt(cipher string, key int) string

with the encryption inverse function form

plain = caesar.Decrypt(caesar.Encrypt(plain, key), key)

Your cipher implementation function is not correct. For example, it fails the Wikipedia: Caesar cipher example:

Example:

func main() {
    var plainText = `THE QUICK BROWN FOX JUMPS OVER THE LAZY DOG`
    fmt.Println(plainText)
    var cipherText = `QEB NRFZH YOLTK CLU GRJMP LSBO QEB IXWV ALD`
    fmt.Println(cipherText)
    var key = -3
    fmt.Println(key)
    fmt.Println(cipher(cipher(plainText, +1, key), -1, key))
}

Output:

THE QUICK BROWN FOX JUMPS OVER THE LAZY DOG
QEB NRFZH YOLTK CLU GRJMP LSBO QEB IXWV ALD
-3
THE QUI&K %ROWN FOX JUMPS OVER THE L$ZY DOG

Your cipher implementation function is orders of magnitude too expensive. Your cipher function versus reasonably efficient package caesar functions:

name      time/op
Cipher-8  9.32µs ± 0%
Caesar-8   502ns ± 0%

name      alloc/op
Cipher-8  2.98kB ± 0%
Caesar-8    192B ± 0%

name      allocs/op
Cipher-8     172 ± 0%
Caesar-8    4.00 ± 0%

Amongst other things, your frequent use of immutable string concatenation (+=) is expensive.

Here is a revised version of your code that addresses my code review issues.

package main

import "fmt"

func caesar(text string, key int) string {
    shift := key % 26
    c := make([]byte, len(text))
    for i, b := range []byte(text) {
        lower := b | 0x20 // ASCII lower case
        if 'a' <= lower && lower <= 'z' {
            base := int('A')
            if b == lower {
                base |= 0x20 // ASCII lower case
            }
            b = byte(base + (int(b)-base+shift+26)%26)
        }
        c[i] = b
    }
    return string(c)
}

func Encrypt(plain string, key int) string {
    return caesar(plain, key)
}

func Decrypt(cipher string, key int) string {
    return caesar(cipher, -key)
}

func main() {
    Plaintext := `THE QUICK BROWN FOX JUMPS OVER THE LAZY DOG`
    fmt.Println(Plaintext)
    Ciphertext := `QEB NRFZH YOLTK CLU GRJMP LSBO QEB IXWV ALD`
    fmt.Println(Ciphertext)
    Key := -3
    fmt.Println(Key)
    fmt.Println(Encrypt(Plaintext, Key))
    fmt.Println(Decrypt(Encrypt(Plaintext, Key), Key))
}

https://go.dev/play/p/7sDXyIcVFpc

THE QUICK BROWN FOX JUMPS OVER THE LAZY DOG
QEB NRFZH YOLTK CLU GRJMP LSBO QEB IXWV ALD
-3
QEB NRFZH YOLTK CLU GRJMP LSBO QEB IXWV ALD
THE QUICK BROWN FOX JUMPS OVER THE LAZY DOG

Coupcoup · Accepted Answer · 2022-07-25 17:34:45Z

I'm going to focus on the cipher function itself for feedback

First, Go includes a range function that lets you iterate/loop over objects and collections directly. If you use range with a string it will return the index and rune so you don't need to worry about using char[i] so many times:

for index, char := range <string> { ... }

Second, you repeat the same check a couple different if statements which is a sign you can likely break that out into a separate function so you only have to write the logic once. I would also reorder it like A <= x <= B so that it's simpler to understand at a glance:

func isLetter(char rune) bool {
 return 'a' <= char && char <= 'z' || 'A' <= char && char <= 'Z'
}

For this case though the Go standard library already contains a unicode package with an isLetter(r rune) function we can use instead of implementing our own.

Lastly, for readability and better comprehension of what your code is trying to achieve you can decompose your algorithm into smaller, clearly named functions. In this case the two main things you're doing are 1. shifting the character and 2. wrapping it back into a specific range if it got shifted out of that. Since each step is relatively straight forward I'd keep them together but if they were more complex you could split them into separate shiftChar and wrapChar functions.

Putting it all together a cleaner version of the cipher function could look something like:

func cipher(text string, direction int, shift int) string {
    var result string

    for _, char := range text {
        result += string(shiftLetterWithWrap(char, direction, shift))
    }

    return result
}

func shiftLetterWithWrap(char rune, direction int, shift int) rune {
    if unicode.IsLetter(char) {
        char += rune(shift * direction)
        
        if !unicode.IsLetter(char) {
            char += rune(direction * -26)
        }
    }

    return char
}

That recreates your original logic but there's presumably a bug if you enter a shift value greater than 26. Ways you could handle that include adding either input validation or changing the shift code to char += rune(shift * direction % 26) — Coupcoup
– Coupcoup, Commented Jul 25, 2022 at 18:11

Stack Exchange Network

caesar cipher decoder and encoder - go

2 Answers 2

You must log in to answer this question.

Hot Network Questions

caesar cipher decoder and encoder - go

2 Answers 2

You must log in to answer this question.

Related

Hot Network Questions