Hot answers tagged

17

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

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

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, &...


11

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 ...


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

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.


9

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 ...


9

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 ...


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

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 ...


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

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 * ...


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

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: // ...


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

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 ...


6

1. declare all 'var' at once instead of var dir string var workers int you can do var ( dir string workers int ) or even better, use local var instead of global var irectly in your main() function dir := flag.String("dir", ".", "directory to search") workers := flag.Int("workers", runtime.NumCPU(), "number of workers") 2. Make sure that ...


5

Though I didn't check the whole program and, according to you, performance is good enough, I do have one performance related suggestion: avoid int64(math.Abs(float64(var1-var2))). Casting between int and float types is not free. Just read about Two's complement for ints and how floats are represented in the IEEE 754 standard. That conversion is not trivial! ...


5

The link is dead and the code doesn't compile, but there is still an obvious issue to critique: Part of the challenge was to not save state with "global variables." In Go, package variables of the package main are typically considered "global variables". However, only a single instance of any package ever exists at run time, so package variables of all ...


5

I would not use SpinWait, since this basically runs a small loop that checks the condition over and over again. This means a lot of CPU cycles are wasted. I would suggest a signalling construct like the ManualResetEventSlim class. You can read about this class and similar constructs on this excellent page about threading in C#. Note: the SpinWait class is ...


5

Early return is better than nesting. For example: var adsPat1 = regexp.MustCompile(`\.txt$`) var adsPat2 = regexp.MustCompile(`_[0-9]{8}\.txt$`) func addDateSuffix(target string, suffix string) error { fis, e := ioutil.ReadDir(target) if e != nil { return e } for _, fi := range fis { if m1 := adsPat1.FindStringSubmatch(fi....


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