Maintainability #58
Maintainability #58
Conversation
costowell
left a comment
costowell
left a comment
There was a problem hiding this comment.
Overall looks good. Still not totally sold on the MATCH thing, but totally open to feedback.
database/poll.go
Outdated
|
|
||
| const POLL_TYPE_SIMPLE = "simple" | ||
| const POLL_TYPE_RANKED = "ranked" | ||
| const MATCH = "$match" |
There was a problem hiding this comment.
I can see where you're (SonarQube is?) going with this, but I'm not so sure this improves maintainability. I think it'd be better to keep this in the query because that's how mongo queries look like i.e.
{ $match: { salary: 9000 } }
I would like more input though, I could be totally crazy.
There was a problem hiding this comment.
I can mark it in SonarQube as a false positive because of the query reasoning
database/poll.go
Outdated
| } | ||
| // Eliminate lowest vote getter | ||
| minVote := 1000000 //the smallest number of votes received thus far (to find who is in last) | ||
| minVote := int(^uint(0) >> 1) //the smallest number of votes received thus far (to find who is in last) |
There was a problem hiding this comment.
This is kind of a clunky way to get the max int (at least I assume this is what's happening). Can you use math.MaxInt instead?
There was a problem hiding this comment.
Yeah, I was avoiding an import for the one line, but it is more readable this way.
Fixes 3/5 maintainability issues outlined in dev's PR to main #56
Notes
Doesn't touch the TODO comments (the other 2 SonarQube issues).
Labeled $match issue (sonarqube wanted this to be a constant) as a false positive