Hot answers tagged

4

If you have m fields and n names then you are iterating O(m * n) times. For all fields and names, circa 60 * 60 = 3600 times. Only loop O(m) times: for field, _ := range m { if !names[field] { delete(m, field) } } With some improvements: package main import ( "encoding/json" "fmt" "regexp" "...


4

Code should be correct, maintainable, robust, reasonably efficient, and, most importantly, readable. Code should be useful and have real value. I found your code hard to read. Your code does not appear to be correct. In Go we write: if err != nil { // error handling return // or continue, etc. } // normal code You confuse everybody, for no obvious ...


3

Go does not have tail recursion: Wikipedia: Tail call. Use iteration rather than recursion. Simplify your sprawling, hard-to-read code. type ListNode struct { Val int Next *ListNode } // Add Two Numbers // https://leetcode.com/problems/add-two-numbers/ func addTwoNumbers(l1 *ListNode, l2 *ListNode) *ListNode { root := &ListNode{} // at end,...


3

I'm looking for it to be peer reviewed with no holds barred critism of my code. Why? to learn new ways of improving my coding techniques and making it much more efficient. Judas Is Back The evaluation criteria for a code review: Code should be correct, maintainable, robust, reasonably efficient, and, most importantly, readable. Code should be useful and ...


3

Disclaimer: I haven't had much exposure to golang. I'm mostly trying to pick up the language by going through random projects. Going over the code you've provided, it seems to be easily followed. A few pointers (questions? concerns?), which might be due to my lack of knowledge: Your min function uses a for loop, where the conditional statement calls len(...


3

You say this is common in all handlers: client, _ := mongo.NewClient(options.Client().ApplyURI("mongodb://127.0.0.1:27017")) ctx, _ := context.WithTimeout(context.Background(), 10*time.Second) err := client.Connect(ctx) if err != nil { panic(err) } defer client.Disconnect(ctx) collection := client.Database("myapp").Collection("questions") So this ...


3

Not sure whether this site is the best place to ask this question, but the answer is quite simple. I'll answer the question, give a few alternatives, pro's and con's and some actual code-review on the little snippet you provided. Do not communicate by sharing memory; instead, share memory by communicating This is the main idea behind channels. When ...


3

You can do this in a single loop, only moving forward through the slice when there are no more duplicates at the current position: func removeAdjacentDups(strings []string) []string { // Iterate over all characters in the slice except the last one for i := 0; i < len(strings)-1; { // Check whether the character next to it is a duplicate ...


3

First, observe that if you receive an out-of-sequence object, you have to potentially keep all the objects from the latest printed sequence number to the received number. Worst case, you receive the object in reverse order and have to keep them all in memory. There is no way around this. So, what you can optimize is first, when you are printing objects you ...


3

working sample code - https://play.golang.org/p/i24iSnpTjtP Really! sample := "This is a rather long line that needs word wrapping to an arbirtary line lenght so it's easier to read it." Check your spelling. sample := "This is a rather long line that needs word wrapping to an arbitrary line length so it's easier to read it." UTF-8 is a ...


3

Your function returns error, but actually you don't return it and exit in the function body when opening the file: func getFileContent(filename string) (records[][] string, err error) { fd, err := os.Open(filename) if err != nil { exit("Problem with opening the file") } reader := csv.NewReader(fd) ...


3

I've majorly written the review comments inline with the code. I could've made the code simple as @thwd suggested; but as you're learning and this is not an actual use case, I'm not changing the logic. Also, to know about buffered channels better, do read this. package main import ( "fmt" "sync" ) // As goroutine could schedule ...


2

From the problem statement: Implement a function pingpong that returns the nth element of the ping-pong sequence without using any assignment statements. You may use the function num_sevens, which you defined in the previous question. Use recursion - the tests will fail if you use any assignment statements. Hint: If you're stuck, first try implementing ...


2

Code should be correct, maintainable, robust, reasonably efficient, and, most importantly, readable. To be certain that the code is correct, carefully read the specification and related documents. Forsyth–Edwards Notation PGN-SPEC Portable Game Notation Algebraic notation (chess) Chess For a code review, compare the specification and the code implementing ...


2

I tried to solve the Problem Euler 67 with Go, because I started to study Go recently. What could be a better approach? Could I use goroutines to increase performance? Code should be correct, maintainable, robust, reasonably efficient, and, most importantly, readable. I'm going to skip a full code review and narrowly focus my code review on the use of ...


2

You have no synchronization for these package variables var shredPath, shredFlag string Therefore, fmt.Println(shredPath, shredFlag) go func() { for i := 0; i < 1000; i++ { shredCmd() } }() for i := 0; i < 1000; i++ { shredCmd() } fmt.Println(shredPath, shredFlag) WARNING: DATA RACE Found 3 data race(s) os.Remove() and commands ...


2

Never user PSEUDO random values for secrets! This could be exploited by an attacker by guessing the seed value, in this case: rand.Seed(time.Now().Unix()) And then generating a possible Cookie value to login. Instead use cryptographically secure implementations, in golang you should use crypto/rand


2

Yes. It's not an absolute rule but generally if you're using more than one if err != nil per function or more then one level of error checking in Go you should refactor. I can't test this because you didn't provide enough code to use it but here is first pass flattened version that reads more clearly: func ReadBook(b *book) (string, error) { ...


2

OK, first off: back when go was first released, a lot of people criticised it for being "too opinionated". The coding style was/is very specific (indentation, brackets, etc... are all standardised). This has turned out to be a great thing. Code written by anyone, provided they've used gofmt looks very similar. This resulted in code being easy to ...


2

I'm reading "Introduction to Algorithms" by CLRS and I can't find an implementation of the pseudo code from the book in golang. Here is my Go implementation of the pseudocode from the book. package main import ( "fmt" "math" ) // Introduction to Algorithms // Third Edition // Cormen, Leiserson, Rivest, Stein func merge(a []float64, p, q, r int)...


2

The first thing I noticed about your folder structure is that all of your internal packages are contained within this app directory. There is no point to this. An internal package, by definition, cannot be imported outside of your project, so any package under internal by definition is part of the application you're building. It's less effort to type import &...


2

The code in the question cannot determine which result corresponds to which operation. Otherwise, the code is correct. Here are two alternatives for improving the code: 1. Eliminate the channel Change the operations to simple functions that return an int. This makes it easier to test and reason about the implementation of the operations. Collect the results ...


2

I guess I might as well then. To prefix that with: It looks fine as is, there's a few details that could be improved, although those also depend on your team and style guide. Also read it in the context of a bigger project, not everything always applies, especially if it's one-off code, etc. I'm gonna assume (since the formatting is a bit off) that you're ...


2

The error from ioutil.TempFile is unhandled, should also return err since everything after it won't work if the temporary file couldn't be created. The strings.Contains check is definitely the least preferred option to check for the kind of error. If at all possible avoid it since it's actually fairly easy for that string to change with, say, a library ...


2

Your code appears to fail on: Example 1: Input: intervals = [[1,3],[2,6],[8,10],[15,18]] Output: [[1,6],[8,10],[15,18]] Explanation: Since intervals [1,3] and [2,6] overlaps, merge them into [1,6]. Output: intervals : [[1 3] [2 6] [8 10] [15 18]] comparing intervals [1 3] and [2 6] comparing intervals [2 6] and [8 10] comparing intervals [8 10] and [...


2

This seems fine to me: it's nice and simple, and I've seen this pattern used "in the wild" before. One thing you could do is, in the MockUserRepo proxying functions, if the function variable is nil, either execute a default implementation, or return a more specific error (or even panic). For example: func (m *MockUserRepo) GetUserByEmail(s string) (...


2

When it comes to "best practices" to mock interfaces, there's a number of things to consider, not in the least: ease of use. Over the years, I've taken to use a mock generator tool. Rather than just implementing the interface in question, GoMock supports a lot of other useful things. You can read more about it here. The way to use it in your case ...


1

Listing in order the things I made note of/changed looking through your code: output chan HHResult is declared but never used in go funcs(output chan HHResult). It's fine that it's not used, the go func has access to Result in the parent function, but you don't need to declare it if it's not needed. The headers map argument is never used either. Looking ...


1

None of your columns are marked non-nullable. This is important for data integrity. duration and date both seem mis-represented as int. You're in PostgreSQL, so read https://www.postgresql.org/docs/current/datatype-datetime.html and then consider instead interval for duration, and date for date


1

You wrote: func doStuff(datachan <-chan map[string]string, reschan chan<- int) { for { data, ok := <-datachan if !ok { log.Print("Channel closed.") break } log.Printf("Data had %d length: %+v", len(data), data) reschan <- len(data) } return } I don't ...


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