Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Calculate the average #2

Closed
wants to merge 1 commit into from

Conversation

pmcelhaney
Copy link

Fixes #1. Shows the average after you're finished voting.

@@ -98,6 +102,9 @@ function RoomCtrl($scope, $routeParams, $timeout, socket) {
$scope.placeholderVotes = voteArr;


var total = _.reduce(_.map(_.pluck($scope.votes, 'vote'), parseFloat), sumOfTwo, 0);

Choose a reason for hiding this comment

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

There's a bug here. parseFloat will choke if anyone has voted ½ or with a non-numeric vote. Non-numeric votes should be excluded and ½ translated to 0.5.

Copy link
Author

Choose a reason for hiding this comment

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

This PR was open almost 7 years before it go a review (and another 1.5 years before I noticed). I'm not mad -- that's amazing!

Copy link

Choose a reason for hiding this comment

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

Amazing but accidental. From my side, it was meant as a bug report through a non-standard channel than a code review. I must have encountered the bug in the wild before I even started looking for the root cause.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry, that came across like a complaint and a I didn't mean for it to be. I just thought it was amusing -- pretty cool actually -- that you saw the PR and responded to it after such a long time. And that it took me such a long time to see that you had responded.

Open source is hard.

Choose a reason for hiding this comment

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

Hard, and honestly amazing (often amusing, too). No irony read or intended.

@barthez
Copy link
Contributor

barthez commented Nov 2, 2020

Average and StdDev calculations are already in master, I believe this PR can be closed.

@jh185173
Copy link

jh185173 commented Nov 2, 2020

Average and StdDev calculations are already in master, I believe this PR can be closed.

Thanks @barhez - Confirming - commit 3de12cc looks like it fixed the calculation bug, and this PR looks obsolete.

@pmcelhaney pmcelhaney closed this Mar 1, 2022
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.

Add an average button
3 participants