# Tag Info

18

Your code is quite nice, and can't really be improved. However, let's put the “generator” back into the code. Go has channels which can be used to write elegant generators/iterators. We spawn of a goroutine that fills the channel with the fibonacci sequence. The main thread then takes as many fibonacci numbers as it needs. So let's write a function ...

16

Note that your code classifies age = 0 as "Adult" which you probably want to be "Infant". (This is because age = 0 is not included in any of your specified ranges and so the last else branch will be executed.) Two completely different solutions will be presented: The clearest and most obvious using switch. An elegant, very compact and faster way using ...

14

I don’t know Go but here are a few general things that I noticed: func between(start, end, value byte) bool { if value > end { return false } else if value < start { return false } return true } As a general pattern, don’t write if condition return true else return false or any permutation thereof. Instead, return the ...

12

encoding/binary package may have what you need. Check this: http://golang.org/pkg/encoding/binary/#example_Read Your code with modified read_int32 function could be: package main import ( "bytes" "encoding/binary" "fmt" ) func read_int32(data []byte) (ret int32) { buf := bytes.NewBuffer(data) binary.Read(buf, binary.LittleEndian, &...

12

Chan sounds like the abbreviated name for Channel and apparently it's a channel. So I'd use Channel or maybe even GoChannel. The most commonly used naming convention I have seen for private members is to prefix them with an underscore. This way you can see at the first glance whether it's a local variable or a class member. This also means you can get rid of ...

12

Rather than tie yourself to only one type (string), you could use the reflect package as well as interfaces to make it somewhat type indifferent. The following is my reworking of your code: package main import "fmt" import "reflect" func in_array(val interface{}, array interface{}) (exists bool, index int) { exists = false index = -1 switch ...

12

This entire answer refers to my rewrite of the asker's code on the Go Playground and is mostly a summary of our discussion on the chat.so Go/Golang room. For reference, I'm reproducing the code below: package main import ( "bufio" "io" "os" "strconv" ) // charSet is a limited bitset to contain all lowercase latin characters (a-z). type ...

11

Without the select operator, you haven't got Go channels - you've just got buffered queues, which are much easier to implement but much less useful. Also, it's important to allow channels with a zero size buffer - in that case the sender should synchronise with the receiver.

11

Review I - package go-fuzzywuzzy After a quick read of your Go code, the Go code for package go-fuzzywuzzy, and the Go documentation, it is reasonable to hope for at least a 60% to 95% improvement in Go performance. For example, $go build fuzzy.go && time ./fuzzy real 0m55.183s user 0m58.858s sys 0m0.944s$ After moving one line in ...

10

Style Use CamelCase only. last_status -> lastStatus, site_json -> siteJSON, etc. Even in constants. UP -> Up or StatusUp, etc. Imports are usually split into groups by source, so separate memcache from stdlib with a newline. Stuttering. E.g. configuration := Configuration{} could be c := Configuration{}. People would search for main() in main.go. So I would ...

10

Instead of checking with an if statement if errStat is null and then returning false: if errStat != nil { return false } return true you can return a Boolean expression: return errStat == nil

9

First let's mention a bug / issue: Your passwords variable is accessed (read and modified) from multiple goroutines without any syncronization: this is a race condition! Primary problem Your memory problem arises from launching a tremendous amount of goroutines. For example when you call compute() with n = 6 (to try passwords with a length of 6), it will ...

9

A vs. B type questions are seldom good questions for a code review. Often the answer is "C". That's true this time too. You don't need a go routine for the "ticker". Your second implementation only waits for 1 second, instead of 2. your "quit" channels don't need to be buffered Why don't you just do: func main() { for t := range time.NewTicker(2 * ...

9

This covers an interesting topic. Great work! Because I am unfamiliar with this area, I utilized your unit testing to ensure changes I make did not break functionality. If they do, I apologize and please let me know. Utilize implicit repetition and iota Rather than manually defining the type and value of nCode, aCode, etc. we an implicitly get the value ...

8

Disclaimer: I don't know Go. Error handling The sendRequest method may fail, returning nil, but the caller in main continues happily as if nothing happened. In fact, looking at the context of the call in main, my first impression was that sendRequest magically always succeeds, which is not the case, and so a bit misleading. Likewise, parseJson may also ...

8

import sanitize "github.com/kennygrant/sanitize" is better written as just plain ol': import "github.com/kennygrant/sanitize" since the rename is redundant. As already mentioned, it's more common to be a bit more explicit with error handling in Go and let utility functions (such as your sendRequest) return any errors for handling "higher up". In ...

8

Bug Your code clearly states: Src: 0, 0 Dst: 0, 5 Excpeted answer: {(0,0), (1,1), (2,2), (2,3), (2,4), (1,5), (0,5)} But, when I run your code, I get: [{0 5} {1 5} {2 4} {2 3} {2 2} {1 1} {0 0}] Everything in reverse order. It is clear that your back-track to reverse the path taken is appending each point to the ans slice, instead of inserting each ...

8

Nice question. Consistent error handling. Regardless of the language, the key to a good program is consistency. Your code combines a combination of panic and error handling mechanisms. You should only be using error values - panicing is an over-the-top reaction and is "not cool". Additionally, you have a copy/paste and error handling problem here too: // ...

8

let me know what I can improve in this code. For a real-world code review, code should be correct, maintainable, reasonably efficient, and, most importantly, readable. Writing code is a process of stepwise refinement. Here's your code. Consider it as a first draft. func BubbleSort(arr []int) []int { keepRunning := true for keepRunning { ...

7

The only weak point is that the implementation is not copy safe nor there exist mechanism for ensuring copy protection. I would hide its underlying type and return as sync.Locker, so it can't be mis-used: type spinLock uint32 func (sl *spinLock) Lock() { for !atomic.CompareAndSwapUint32((*uint32)(sl), 0, 1) { runtime.Gosched() //without this it ...

7

There's not really a one "right" way of doing things, and all of the others are wrong. However, there's a number of things that are considered good practice, and WRT to those, your code can do with a bit of TLC. To that end, I'll just run through your code line by line, leaving comments on both style, and things I deem to be missing, hopefully ending up ...

6

This improves slightly on Keeth's answer by making the source of the magic numbers more clear. func (r *rot13Reader) Read(p []byte) (n int, err error) { n, err = r.r.Read(p) for i := 0; i < n; i++ { c := p[i] switch { case 'a' <= c && c <= 'z': p[i] = (c-'a'+13)%26 + 'a' case 'A' <= c &...

6

First, let's rewrite your code in idiomatic Go. // For loop using a for clause func NewRectangleF(height, width int, value byte) [][]byte { r := make([][]byte, 0, height) for i := 0; i < height; i++ { r = append(r, bytes.Repeat([]byte{value}, width)) } return r } // For loop using a range clause func NewRectangleR(height, width ...

6

You have correctly noticed that this isn't extremely idiomatic. However, this code also has some issues that are completely language independent: You have a large amount of magic numbers. Either explain them with a short comment, or (preferably) calculate them. Let the compiler do constant folding; don't do it yourself. Your code does not have a single ...

6

Inside SendRequst you are calling SendRequest again, which will make your stack full of recursive calls.

6

This seems like a good solution except for one thing: func power(a, b int) int { return a ^ b } The ^ operator doesn't do what you think it does. It's a bitwise xor operator. There is no power operator in Go, there's only math.Pow for float64s. On a side note, I personally would create a type for operators: type Op func(int, int) int It's easier to ...

6

There are a few things I would point out as being poor go style. The two different nesting loops is where I would start: for i := range A { for j := i + 1; j < len(A); j++ { The outer loop does a index-only range on the A slice, which in itself is not a problem, but logically it is for i := 0; i < len(A); i++ {. Again, this is not really a ...

6

Your code is neat, and structured just like I would expect to see Java ;-) - separate files for each "class". In your case, though, the separation in to files is probably over the top, and if you are talking idiomatic go, I would not expect to see separate files for the NotGate and the XorGate. There is a small bug in your code, you use: fmt.Print("%d: {%t,...

6

Combine the mutex with the thing it is locking into a single struct. Often mutex elements of a struct are unnamed, since you only need to lock and unlock the whole thing type SafeMap struct { sync.Mutex URLs map[string]bool } which you can then use like so: url := "http://some_example.com" fetched := SafeMap{URLs:map[string]bool{}} fetched.Lock() ...

6

Bugs Your code does have a problem with the concurrency, and it's a big one.... your buffered channels are not going to be able to complete the system. The issue is the unbuffered resc and errc channels: resc, errc := make(chan string), make(chan error) Your code attempts to loop through a million input records, but your concurrency is set relatively low ...

Only top voted, non community-wiki answers of a minimum length are eligible