Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

I have optimized the code #32

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
58 changes: 58 additions & 0 deletions Go/shortUrl.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/**
* ShortURL: Bijective conversion between natural numbers (IDs) and short strings
* Licensed under the MIT License (https://opensource.org/licenses/MIT)
*
* ShortURL::encode() takes an ID and turns it into a short string
* ShortURL::decode() takes a short string and turns it into an ID
*
* Features:
* + large alphabet (51 chars) and thus very short resulting strings
* + proof against offensive words (removed 'a', 'e', 'i', 'o' and 'u')
* + unambiguous (removed 'I', 'l', '1', 'O' and '0')
**/
package ShortURL

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where are the unit tests?


import (
"fmt"
"strings"
)

const (
Alphabets = "23456789bcdfghjkmnpqrstvwxyzBCDFGHJKLMNPQRSTVWXYZ-_"
Base = len(Alphabets)
)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to export Alphabets and Base?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the other implementations (at least PHP,Java) have them public, so for the compatibility sake so Go too?

I would imagine it would be nice to have them as public, maybe some web servers need to do some white listing, or dynamic routing based on them?


// reverseChar Utility to reverse string with only UTF8
func reverseChars(s string) string {
bytes := []byte(s)
for i, j := 0, len(bytes)-1; i < j; i, j = i+1, j-1 {
bytes[i], bytes[j] = bytes[j], bytes[i]
}
return string(bytes)
}

// Encode: given a generated number, get the URL back
func Encode(n int) string {
sb := strings.Builder{}
for n > 0 {
sb.WriteByte(Alphabets[n%Base])
n /= Base
}
// We know that Alphabet set is UTF8, so we will use reverseChars.
return reverseChars(sb.String())
}

// Decode: given a URL(path), the decoder decodes it to a unique number.
func Decode(path string) (int, error) {
n := 0
for _, c := range path {
i := strings.Index(Alphabets, string(c))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can use strings.IndexRune, you already have the rune as c, this way you can get rid of the (string) cast.

if i < 0 {
return 0, fmt.Errorf("Invalid character %s in input %s", string(c), path)
} else {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need for else, also it is not idiomatic from what I know.

n = n*Base + i
}

}
return n, nil
}