Hot answers tagged

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


17

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

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


13

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


13

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


12

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


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

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


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

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


9

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


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

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


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

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


7

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


7

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


7

Your code has a few items in it which concern me in a few small ways, and, in general, while I think your code is OK functionally, it could be done better (I am biased, I have done similar things, and ended up doing it differently). First up, my concerns... Timestamp function while it is a 1-liner, I would still have a method to encapsulate the time ...


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

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


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

I would not implement method UnmarshalJSON as it is not really necessary. Instead, I would create Audio and Photo types and put them as fields into Attachment type. It would be easier to reference data as field than get it as a value from map. package main import ( "encoding/json" "fmt" "log" "strings" ) type Response struct { Id ...


6

Version control system principle Your "version control system" principle is a little strange for me. You don't store which files belong to a revision (ID) and so for example if you add a file and you remove it later, the db file will have no knowledge of it. And so if you perform a revert operation, it will not delete previously committed instances of that ...


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


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