HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Condorcet voting method in OOP Python

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
votingmethodpythoncondorcetoop

Problem

For learning purposes, I made this little class which determines the Condorcet winner of a rank voting election (basically, a Condorcet winner is a candidate that would beat every other candidate in a head-to-head election).

This code does work, but I'd like to learn two-three things by posting it here:

  • Is this class written in a correct Pythonic way?



  • Do you see any ways to improve this code?



  • (bonus) If there are voting specialists among you, could you tell if that way of going through a Condorcet algorithm seems correct?



Condorcet.py:

```
import itertools

class Condorcet:
"""
Gives Concorcet winner from a list of votes
"""
def __init__(self, filename):
self.candidates = set()
self.scores = dict() # score for each permutation of two candidates
self.results = dict() # winner of each pair of candidates
self.winner = None # Condorcet winner of the election
self.voters = list() # ranking of each voter
self.filename = filename

def process(self):
self._get_data_from_file()
self._build_dict()
self._matches_candidates()
self._elect_winner()
return self.winner

def _get_data_from_file(self):
with open(self.filename, encoding='utf-8') as file:
for lines in file:
# currently hard-coding the number of candidates at 4
# needs fixing but it's not my point here
# (more a question for StackOverflow actually)
(one, two, three, four) = lines.split(None, 4)
self.voters.append((one, two, three, four))

def _build_dict(self):
"""
Builds a dictionary of scores
for each permutation of two candidates
"""
for voting in self.voters:
for candidate in voting:
self.candidates.add(candidate)
for pair in list(itertools.permutations(voting, 2)):
if pair not in self.

Solution

The purpose of your class is to perform a single computation: given filename, determine winner. That's a task for a function; there is no need for a class. You can turn each method to an independent function that takes one or two arguments and returns one or two values instead of accessing instance attributes, and provide this function to orchestrate the whole computation:

def concordet_winner(filename):
    voters = _get_data_from_file(filename)
    candidates, scores = _build_dict(voters)
    results = _matches_candidates(candidates, scores)
    return _elect_winner(candidates, results)


Now, reading this code gives a clear picture how the data flows through the computation.

For the purpose of learning OOP, you could consider a slightly different exercise: instead of having all votes in a file, create a class with a vote method that takes in an individual voter's vote, and a winner property that updates automatically after every vote.

Code Snippets

def concordet_winner(filename):
    voters = _get_data_from_file(filename)
    candidates, scores = _build_dict(voters)
    results = _matches_candidates(candidates, scores)
    return _elect_winner(candidates, results)

Context

StackExchange Code Review Q#42359, answer score: 5

Revisions (0)

No revisions yet.