1
\$\begingroup\$

I implemented the caesar cipher along with a writer/reader and a command line interface.

package caesar

import "fmt"

// ShiftOutOfRangeError denotes a shift which is as great or greater than the character count of the alphabet, zero or negative.
type ShiftOutOfRangeError int

// Error return the formated error message.
func (s ShiftOutOfRangeError) Error() string {
    return fmt.Sprintf("caesar: shift %d is out of range", s)
}

// Cipher denotes the caesar cipher with mutable shift.
type Cipher struct {
    Shift int
}

const (
    // letterCount defines the count of characters in the latin alphabet.
    letterCount = 26
    minShift    = -letterCount - 1
    maxShift    = letterCount - 1
)

// NewCipher initializes a new Cipher instance.
// If there is an error, it will be of type ShiftOutOfRangeError.
func NewCipher(shift int) (Cipher, error) {
    if shift < minShift {
        return Cipher{minShift}, ShiftOutOfRangeError(shift)
    } else if shift > maxShift {
        return Cipher{maxShift}, ShiftOutOfRangeError(shift)
    }
    return Cipher{shift}, nil
}

// Rotate replaces the character with that one the provided places down the alphabet.
// Ignores non-ASCII letters.
func (cipher Cipher) Rotate(c byte) byte {
    // ignore non-ASCII letters
    if c < 'A' || c > 'z' {
        return c
    }

    isUpper := c < 'a'
    c += byte(cipher.Shift)
    if (isUpper && c > 'Z') || c > 'z' {
        c -= letterCount
    } else if c < 'A' || (!isUpper && c < 'a') {
        c += letterCount
    }
    return c
}

// RotateText replaces all characters with that one the provided places down the alphabet.
func (cipher Cipher) RotateText(s string) string {
    b := []byte(s)
    cipher.RotateBytes(b)
    return string(b)
}

// RotateBytes replaces all as characters interpreted bytes with that one the provided places down the alphabet.
func (cipher Cipher) RotateBytes(b []byte) {
    for i := 0; i < len(b); i++ {
        b[i] = cipher.Rotate(b[i])
    }
}

package caesar

import "io"

// Reader rotates all read bytes with the caesar cipher.
type Reader struct {
    r      io.Reader
    Cipher Cipher
}

// NewReader initalizes a new Reader instance.
func NewReader(r io.Reader, c Cipher) Reader {
    return Reader{r, c}
}

// Read rotates the by the underlying io.Reader read bytes.
func (r Reader) Read(p []byte) (n int, err error) {
    n, err = r.r.Read(p)
    r.Cipher.RotateBytes(p)
    return
}

package caesar

import "io"

// Writer rotates all written bytes with the caesar cipher.
type Writer struct {
    w      io.Writer
    Cipher Cipher
}

// NewWriter initializes a new Writer instance.
func NewWriter(w io.Writer, c Cipher) Writer {
    return Writer{w, c}
}

// Write forwards the rotated bytes to the underlying io.Writer.
func (w Writer) Write(p []byte) (int, error) {
    w.Cipher.RotateBytes(p)
    return w.w.Write(p)
}
package main

import (
    "fmt"
    "io"
    "os"
    "strconv"

    "github.com/r3turnz/caesar"
)

func fatalf(format string, a ...interface{}) {
    fmt.Fprintf(os.Stderr, format, a...)
    os.Exit(1)
}

func printHelp() {
    fmt.Printf("Usage: %s -h shift\n"+
        "Read from stdin and write with caesar cipher encoded characters to stdout.\n", os.Args[0])
}

func main() {
    if len(os.Args) > 1 && ("-h" == os.Args[1] || "--help" == os.Args[1]) {
        printHelp()
        os.Exit(1)
    }
    var shift int
    if len(os.Args) > 1 {
        var err error
        shift, err = strconv.Atoi(os.Args[1])
        if err != nil {
            fatalf("Shift %q is not a valid integer", os.Args[1])
        }
    } else {
        shift = 13
    }

    cipher, err := caesar.NewCipher(shift)
    if err != nil {
        err := err.(caesar.ShiftOutOfRangeError)
        fatalf("Shift %q is out of range\n", err)
    }
    out := caesar.NewWriter(os.Stdout, cipher)
    io.Copy(out, os.Stdin)
}

I am quite new to go and any improvment suggestions are welcome!

\$\endgroup\$
8
  • \$\begingroup\$ You should write some automated tests to ensure that your code works as expected. It doesn't do what I expect at least. Cipher('Z', 1) should be 'A', but it is actually 'Y'. \$\endgroup\$ Commented Aug 23, 2017 at 20:58
  • \$\begingroup\$ After some testing the current implementation only seems to work with shift of 13. \$\endgroup\$ Commented Aug 24, 2017 at 0:00
  • \$\begingroup\$ I rewrote the implementation and it now works with all shifts. \$\endgroup\$ Commented Aug 24, 2017 at 20:08
  • \$\begingroup\$ Please read codereview.stackexchange.com/help/someone-answers, the section "What should I not do" applies here. \$\endgroup\$ Commented Aug 24, 2017 at 20:13
  • \$\begingroup\$ And still, your Caesar "encryption" is not reversible. When you "encrypt" the string "[" and then decrypt it again, you don't get the original text. \$\endgroup\$ Commented Aug 24, 2017 at 20:20

1 Answer 1

2
\$\begingroup\$

The most important property of a program is that it is correct. For the Caesar Cipher, Latin letters are rotated, in either direction, by a fixed shift amount. Decoding is the reverse of encoding.

Your algorithm looks suspicious. I tested your algorithm against my algorithm. Encoding "Hello, 世界" for shift +25, your algorithm gives "a~\u0085\u0085\u0088, 世界" and my algorithm gives "Gdkkn, 世界".

Your algorithm:

type ShiftOutOfRangeError int32

func (s ShiftOutOfRangeError) Error() string {
    return fmt.Sprintf("shift %d is out of range", int32(s))
}

// Cipher replaces the character with that one the provided places down the alphabet.
func Cipher(r rune, shift int32) (rune, error) {
    if shift < 1 || shift > 25 {
        return r, ShiftOutOfRangeError(shift)
    }
    // ignore non-ASCII letters
    if r < 'A' || r > 'z' {
        return r, nil
    }

    if r >= 'a' && r < 'a'+shift || r >= 'A' && r < 'A'+shift {
        r += shift
    } else {
        r -= shift
    }
    return r, nil
}

Here's my algorithm:

// Rotate Latin letters by the shift amount.
func rotate(text string, shift int) string {
    shift = (shift%26 + 26) % 26 // [0, 25]
    b := make([]byte, len(text))
    for i := 0; i < len(text); i++ {
        t := text[i]
        var a int
        switch {
        case 'a' <= t && t <= 'z':
            a = 'a'
        case 'A' <= t && t <= 'Z':
            a = 'A'
        default:
            b[i] = t
            continue
        }
        b[i] = byte(a + ((int(t)-a)+shift)%26)
    }
    return string(b)
}

// Encode using Caesar Cipher.
func Encode(plain string, shift int) (cipher string) {
    return rotate(plain, shift)
}

// Decode using Caesar Cipher.
func Decode(cipher string, shift int) (plain string) {
    return rotate(cipher, -shift)
}

Your updated algorithm is not correct. Did you test it?

For example,

package caesar_test

import (
    "github.com/r3turnz/caesar"
    "testing"
)

func TestEncodeDecode(t *testing.T) {
    text := "Hello, 世界, Aa-Zz"
    for shift := -26 - 25; shift <= +25+26; shift++ {
        enc := caesar.Cipher{Shift: shift}
        dec := caesar.Cipher{Shift: -shift}
        encdec := dec.RotateText(enc.RotateText(text))
        if encdec != text {
            t.Errorf("got: %q want: %q shift: %d\n", encdec, text, shift)
        }
    }
}

Output:

$ go test github.com/r3turnz/caesar
--- FAIL: TestEncodeDecode (0.00s)
    peter_test.go:15: got: "/ello, 世界, (a-Zz" want: "Hello, 世界, Aa-Zz" shift: -51
    peter_test.go:15: got: "0ello, 世界, )a-Zz" want: "Hello, 世界, Aa-Zz" shift: -50
    peter_test.go:15: got: "1ello, 世界, *a-Zz" want: "Hello, 世界, Aa-Zz" shift: -49
    peter_test.go:15: got: "2ello, 世界, +a-Zz" want: "Hello, 世界, Aa-Zz" shift: -48
    peter_test.go:15: got: "3ello, 世界, ,a-Zz" want: "Hello, 世界, Aa-Zz" shift: -47
    peter_test.go:15: got: "4ello, 世界, -a-Zz" want: "Hello, 世界, Aa-Zz" shift: -46
    peter_test.go:15: got: "5ello, 世界, .a-Zz" want: "Hello, 世界, Aa-Zz" shift: -45
    peter_test.go:15: got: "6ello, 世界, /a-Zz" want: "Hello, 世界, Aa-Zz" shift: -44
    peter_test.go:15: got: "7ello, 世界, 0a-Zz" want: "Hello, 世界, Aa-Zz" shift: -43
    peter_test.go:15: got: "8ello, 世界, 1a-Zz" want: "Hello, 世界, Aa-Zz" shift: -42
    peter_test.go:15: got: "9ello, 世界, 2a-Zz" want: "Hello, 世界, Aa-Zz" shift: -41
    peter_test.go:15: got: ":ello, 世界, 3a-Zz" want: "Hello, 世界, Aa-Zz" shift: -40
    peter_test.go:15: got: ";ello, 世界, 4a-Zz" want: "Hello, 世界, Aa-Zz" shift: -39
    peter_test.go:15: got: "<ello, 世界, 5a-Zz" want: "Hello, 世界, Aa-Zz" shift: -38
    peter_test.go:15: got: "=ello, 世界, 6a-Zz" want: "Hello, 世界, Aa-Zz" shift: -37
    peter_test.go:15: got: ">ello, 世界, 7a-Zz" want: "Hello, 世界, Aa-Zz" shift: -36
    peter_test.go:15: got: "?ello, 世界, 8a-Zz" want: "Hello, 世界, Aa-Zz" shift: -35
    peter_test.go:15: got: "@ello, 世界, 9a-Zz" want: "Hello, 世界, Aa-Zz" shift: -34
    peter_test.go:15: got: "Hello, 世界, :a-Zz" want: "Hello, 世界, Aa-Zz" shift: -33
    peter_test.go:15: got: "Hello, 世界, ;a-Zz" want: "Hello, 世界, Aa-Zz" shift: -32
    peter_test.go:15: got: "Hello, 世界, <a-Zz" want: "Hello, 世界, Aa-Zz" shift: -31
    peter_test.go:15: got: "Hello, 世界, =a-Zz" want: "Hello, 世界, Aa-Zz" shift: -30
    peter_test.go:15: got: "Hello, 世界, >a-Zz" want: "Hello, 世界, Aa-Zz" shift: -29
    peter_test.go:15: got: "Hello, 世界, ?a-Zz" want: "Hello, 世界, Aa-Zz" shift: -28
    peter_test.go:15: got: "Hello, 世界, @a-Zz" want: "Hello, 世界, Aa-Zz" shift: -27
    peter_test.go:15: got: "Hello, 世界, Aa-Z{" want: "Hello, 世界, Aa-Zz" shift: 27
    peter_test.go:15: got: "Hello, 世界, Aa-Z|" want: "Hello, 世界, Aa-Zz" shift: 28
    peter_test.go:15: got: "Hello, 世界, Aa-Z}" want: "Hello, 世界, Aa-Zz" shift: 29
    peter_test.go:15: got: "Hello, 世界, Aa-Z~" want: "Hello, 世界, Aa-Zz" shift: 30
    peter_test.go:15: got: "Hello, 世界, Aa-Z\u007f" want: "Hello, 世界, Aa-Zz" shift: 31
    peter_test.go:15: got: "Hello, 世界, Aa-Z\x80" want: "Hello, 世界, Aa-Zz" shift: 32
    peter_test.go:15: got: "Hello, 世界, Aa-Z\x81" want: "Hello, 世界, Aa-Zz" shift: 33
    peter_test.go:15: got: "Hello, 世界, Aa-Z\x82" want: "Hello, 世界, Aa-Zz" shift: 34
    peter_test.go:15: got: "Hello, 世界, Aa-Z\x83" want: "Hello, 世界, Aa-Zz" shift: 35
    peter_test.go:15: got: "Hello, 世界, Aa-Z\x84" want: "Hello, 世界, Aa-Zz" shift: 36
    peter_test.go:15: got: "Hello, 世界, Aa-Z\x85" want: "Hello, 世界, Aa-Zz" shift: 37
    peter_test.go:15: got: "Hell{, 世界, Aa-Z\x86" want: "Hello, 世界, Aa-Zz" shift: 38
    peter_test.go:15: got: "Hell|, 世界, Aa-Z\x87" want: "Hello, 世界, Aa-Zz" shift: 39
    peter_test.go:15: got: "Hell}, 世界, Aa-Z\x88" want: "Hello, 世界, Aa-Zz" shift: 40
    peter_test.go:15: got: "He{{~, 世界, Aa-Z\x89" want: "Hello, 世界, Aa-Zz" shift: 41
    peter_test.go:15: got: "He||\u007f, 世界, Aa-Z\x8a" want: "Hello, 世界, Aa-Zz" shift: 42
    peter_test.go:15: got: "He}}\x80, 世界, Aa-Z\x8b" want: "Hello, 世界, Aa-Zz" shift: 43
    peter_test.go:15: got: "He~~\x81, 世界, Aa-Z\x8c" want: "Hello, 世界, Aa-Zz" shift: 44
    peter_test.go:15: got: "He\u007f\u007f\x82, 世界, Aa-Z\x8d" want: "Hello, 世界, Aa-Zz" shift: 45
    peter_test.go:15: got: "He\x80\x80\x83, 世界, Aa-Z\x8e" want: "Hello, 世界, Aa-Zz" shift: 46
    peter_test.go:15: got: "He\x81\x81\x84, 世界, Aa-Z\x8f" want: "Hello, 世界, Aa-Zz" shift: 47
    peter_test.go:15: got: "H{\x82\x82\x85, 世界, Aa-Z\x90" want: "Hello, 世界, Aa-Zz" shift: 48
    peter_test.go:15: got: "H|\x83\x83\x86, 世界, Aa-Z\x91" want: "Hello, 世界, Aa-Zz" shift: 49
    peter_test.go:15: got: "H}\x84\x84\x87, 世界, Aa-Z\x92" want: "Hello, 世界, Aa-Zz" shift: 50
    peter_test.go:15: got: "H~\x85\x85\x88, 世界, Aa-Z\x93" want: "Hello, 世界, Aa-Zz" shift: 51
FAIL
FAIL    github.com/r3turnz/caesar   0.004s
$
\$\endgroup\$
3
  • \$\begingroup\$ Thank you! Check out my updated algorithm. I think I wrote a very clean and working one. \$\endgroup\$ Commented Aug 24, 2017 at 9:28
  • \$\begingroup\$ @R3turnz: Your updated algorithm doesn't work. See my revised answer. \$\endgroup\$ Commented Aug 24, 2017 at 14:10
  • \$\begingroup\$ The test is buggy, not the implementation. It makes no sense to rotate with a shift greater than 25. Shift 26 would be the indentical character and shift 27 is identical to shift 1. \$\endgroup\$ Commented Aug 24, 2017 at 20:07

You must log in to answer this question.