First submission #117
First submission #117
Conversation
|
There was a problem hiding this comment.
Code Review:
Its minor, but Im not sure why you make your imports multi-line capable when all you need is "fmt". This just can make your code cluttered.
In challenge_2, you make variables singles and repeats. Why? If all you're looking for is unique character or number, why stash the repeated and delete a character or number from singles? Surely there is a better way to do this. Ill go ahead and merge your PR since we lack a lot of Go submissions.
But good job with everything else! I like your style os code and your use of Go!
|
|
||
| func FindSingleton(int_array []int) (found int) { | ||
| singles := make(map[int]int) | ||
| repeats := make(map[int]int) |
There was a problem hiding this comment.
Any reason you can't just have one map of map[int]int and just loop over it at the end to find the (or a) single and return that? One less map allocation?
There was a problem hiding this comment.
This is one less loop iteration at the cost of one allocation. Different trade-offs. Realistically, these maps are being used as Sets and repeats doesn't need to bother counting.
| } | ||
| } | ||
| for k, _ := range singles { | ||
| found = k |
There was a problem hiding this comment.
Might as well exit the for loop when you find one since you'll just be reassigning found each time you find one and it doesn't look like it matters what one you return.
There was a problem hiding this comment.
If I had the return within the loop, I would still need the end the function return, since the loop might contain nothing. It doesn't gain anything since this loop should only iterate one value, per spec. If I updated to have both return statements, I would also change the return value to (int, error) and rework the loop so an error could be returned on no single or multiple singles. Not adding the error return was just my laziness. ;)
| "fmt" | ||
| ) | ||
|
|
||
| func FindMajority(int_array []int) int { |
There was a problem hiding this comment.
Go idioms is to use smaller naming conventions. This is also a slice, not an array. Maybe ints
There was a problem hiding this comment.
ints would be good as it is still descriptive enough.
| } | ||
|
|
||
| func (t *TreeNode) String() string { | ||
| l := "" |
There was a problem hiding this comment.
I think you'd more typically see
var l string
var r stringYou could do the same for the numbers as above but it's mainly in strings where it's a bit odd to use shorthand to create an empty value string when it's already that way with var
There was a problem hiding this comment.
Either works fine. I find explicit initialization to be more descriptive but each to his own.
There was a problem hiding this comment.
(Also yes I do appreciate the irony of 1-letter variable names when talking about descriptiveness. Once again, laziness in my part to crank out a simple problem, lol. I'm glad code standards will be implemented in the repo.)
No description provided.