Go Implementation

[sandeepkalra]

Go Impl.

[railsmechanic]

Is this println intended?

[sandeepkalra]

No, that was just for debugging. Best Regards, Sandeep Kalra

…

On Tue, Jul 11, 2017 at 2:02 AM, Railsmechanic ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In Go/shortUrl.go <#19 (comment)>: > + "fmt" + "strings" +) + +const ( + Alphabets = "23456789bcdfghjkmnpqrstvwxyzBCDFGHJKLMNPQRSTVWXYZ-_" + Base = len(Alphabets) +) + +type Codec struct{} + +func (c *Codec) Encode(n int) string { + var s string + for n > 0 { + c := string(Alphabets[n%Base]) + fmt.Println(c) Is this println intended? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#19 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFTvwqmw4ZRAcNtI_dSS3g2qu8bp9bDGks5sMx4OgaJpZM4NsUCS> .

[ocram]

Oh, totally forgot about this pull request. Sorry!

Are the Codec and InitShorteningCodec really necessary? What do they do?

Thanks for your work, @sandeepkalra!

[sandeepkalra]

Yes, the InitShorteningCodec is required. It returns the instance/object of the structure that has Encode and Decode functions.

[sandeepkalra]

I coded as per GoLang paradigm . The library offers a instance of itself called " Codec{} " by using "InitShorteningCodec()" that has access functions "Encode()" and "Decode()".

[bgadrian]

Encode function is not optimal, it allocates a new string for each step, you should use a bytes.buffer or a Strings.builder

[sandeepkalra]

I wrote this before go ver1.10, and strings.builder wasn't there. I did try to play with it today after your comments, but the benchmark results didn't show anything promising for me to change.

Please see

===== code ====
// Note: This is not correct impl.,
// I should have to pre-append, but I am only measuring the
// time here, so let it run this way.

func (c *Codec) EncodeOpti(n int) string {
sb := strings.Builder{}
sb.Grow(n + 1)
for n > 0 {
sb.WriteByte(Alphabets[n%Base])
n /= Base
}
return sb.String()
}

func (c *Codec) Encode(n int) string {
var s string
for n > 0 {
s = string(Alphabets[n%Base]) + s
n /= Base
}
return s
}

=== benchmark ===
func BenchmarkEncode1(b *testing.B) {
en := InitShorteningCodec()
for i := 0; i < 1000000; i++ {
en.EncodeOpti(i)
}
}
func BenchmarkEncode2(b *testing.B) {
en := InitShorteningCodec()
for i := 0; i < 1000000; i++ {
en.Encode(i)
}
}
===== benchmark results ====

{Go}-> go test -bench=Encode2 (calling old code)
goos: darwin
goarch: amd64
BenchmarkEncode2-8 2000000000 0.08 ns/op
PASS
ok _/Users/kalra/projects/ShortURL/Go 1.672s
{Go}-> go test -bench=Encode1 (calling new code)
goos: darwin
goarch: amd64
BenchmarkEncode1-8 1 41155475176 ns/op
PASS
ok _/Users/kalra/projects/ShortURL/Go 41.168s

[sandeepkalra]

I think my post earlier is a little too early. I have not studied strings.builder correctly I did another small change, and removed the sb.Grow(), and it seems to now run in 1/3rd time from previous. I will make changes to my code after I have full understanding of string.Builder library.

[ocram]

Thanks, @bgadrian, and thanks for testing this and developing a modified implementation, @sandeepkalra!

So this is now our third candidate for the Go implementation: #32