Skip to content

Conversation

@shaeespring
Copy link
Contributor

Fixes 3/5 maintainability issues outlined in dev's PR to main #56

  • Reduces the complexity of the calculateRankedChoice function (check my math, I might need helper functions to get the number lower, or it might just be fine)
  • Fixes a variable name
  • Creates a constant for a commonly used variable

Notes

Doesn't touch the TODO comments (the other 2 SonarQube issues).

Copy link
Contributor

@costowell costowell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good. Still not totally sold on the MATCH thing, but totally open to feedback.


const POLL_TYPE_SIMPLE = "simple"
const POLL_TYPE_RANKED = "ranked"
const MATCH = "$match"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
// 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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants