Skip to content

Feedback#3

Open
CheezItMan wants to merge 1 commit intohalahaddad1:masterfrom
CheezItMan:patch-3
Open

Feedback#3
CheezItMan wants to merge 1 commit intohalahaddad1:masterfrom
CheezItMan:patch-3

Conversation

@CheezItMan
Copy link

Election Time

Requirement Comments
Well formatted code You are using variable names like g, m, and d, these are not terribly meaningful. You also have some extraneous spacing like on line 36. You are also using 4-space indentation instead of 2-space indents.
Reads in votes 👍
Outputs the total for each candidate 👍
Determines the winner 👍
Optional Handle ties for a winner appropriately NA
Optional Handle grammar of vote summary saying vote or votes appropriately NA
Optional Handle write in votes NA
Optional Consider how to handle more than 10 votes NA
Optional Consider how to handle more than 3 candidates NA

Summary

Nice work, you hit all the basic learning goals here. Consider if you used a hash with the candidate names as keys and the values being the vote count. Would that simplify things?

# Election Time

| Requirement | Comments
|---|---
| Well formatted code |   You are using variable names like `g`, `m`, and `d`, these are not terribly meaningful.  You also have some extraneous spacing like on line 36.   You are also using 4-space indentation instead of 2-space indents. 
| Reads in votes |  👍 
| Outputs the total for each candidate |  👍 
| Determines the winner |  👍 
| `Optional` Handle ties for a winner appropriately |  NA
| `Optional` Handle grammar of vote summary saying vote or votes appropriately | NA
| `Optional` Handle write in votes | NA
| `Optional` Consider how to handle more than 10 votes | NA
| `Optional` Consider how to handle more than 3 candidates | NA

## Summary

Nice work, you hit all the basic learning goals here.  Consider if you used a hash with the candidate names as keys and the values being the vote count.  Would that simplify things?
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.

1 participant