1

First and foremost this is an assignment for my college class. Please do not hack the answer out for me as it will not help me help myself. I'm merely looking for an experienced point of view as to why I have hit a brick wall on something that seems easy.

What I have done: I have an Election class that will add Candidate objects to an ArrayList of type Candidate. A Candidate object has the following attributes, a name, id, and total number of votes (that starts at zero, of course). These Candidate objects are created and stored into an Candidate[] within the main client. The Candidate object's are then added to the Arraylist of type Candidate - located in the Election class.

My Problem: The votes for a Candidate are read from a text file that consists of numbers 1 -24. Each vote corresponds to the id of a Candidate. In the countVotes method located in the Election class I'm reading the text file containing the votes. I am then trying to compare the votes read from the text file in real-time to a Candidate's id. If a vote and Id are equal then increment count to get total number of votes for a Candidate. However, this method will only work for one Candidate id at a time. For example if I fed the method call countVotes(readFile, cands[1]) it will work for only the specific Candidate. I know that I should probably store the votes in another ArrayList and then do comparison using two ArrayLists. I'm looking for insight on how to go about comparing the votes read from the text file to each Candidate's id and then setting the total number of votes counted for each Candidate.

Here is the Candidate class:

package problem_one;

public class Candidate <E> {
    protected String name;
    protected int id;
    protected int votes;

    public Candidate(String name, int id){
        this.name = name;
        this.id = id;
        votes = 0;
    }

    public String getName() {
        return name;
    }

    public int getId() {
        return id;
    }

    public void setVotes(int votes) {
        this.votes = votes;
    }

    public int getVotes(){
        return votes;
    }

    public String toString(){
        String display = "\nCandidate: " + getName() + "\nID: " + getId() + "\nVotes: " + getVotes() + "\n";

        return display;
    }
}

Here is the Election Class:

package problem_one;

import java.util.ArrayList;
import java.util.Scanner;

public class Election<E>{

    @SuppressWarnings("rawtypes")
    protected ArrayList<Candidate> ballot = new ArrayList<Candidate>();

    public void addCandidate(Candidate<E> cand){
        ballot.add(cand);
    }

    public void countVotes(Scanner readFile, Candidate cand){

        Integer vote;
        int count = 0;

            while(readFile.hasNextInt()){
                vote = readFile.nextInt();

                if(vote.equals(cand.getId())){
                    count++;
                    cand.setVotes(count);
                }
            }
    }

    public String toString(){
        String display = "";

        for(Candidate cand : ballot){
            display += cand;
        }
        return display;
    }
}

Here is the main client:

package pa2_clients;

import java.io.File;
import java.io.FileNotFoundException;
import java.util.ArrayList;
import java.util.Scanner;

import problem_one.Candidate;
import problem_one.Election;

public class problem_one_client {

    public static void main(String[] args) {
        Election<Object> elect = new Election<Object>();
        File file = new File("votes");
        Scanner readFile;

        Candidate[] cands = new Candidate[24];

        cands[0] = new Candidate<Object>("Washingtion", 1);
        cands[1] = new Candidate<Object>("Adams", 2);
        cands[2] = new Candidate<Object>("Jefferson", 3);
        cands[3] = new Candidate<Object>("Madison", 4);
        cands[4] = new Candidate<Object>("Monroe", 5);
        cands[5] = new Candidate<Object>("Quincy Adams", 6);
        cands[6] = new Candidate<Object>("Jackson", 7);
        cands[7] = new Candidate<Object>("Van Buren", 8);
        cands[8] = new Candidate<Object>("Harrision", 9);
        cands[9] = new Candidate<Object>("Tyler", 10);
        cands[10] = new Candidate<Object>("Polk", 11);
        cands[11] = new Candidate<Object>("Taylor", 12);
        cands[12] = new Candidate<Object>("Fillmore", 13);
        cands[13] = new Candidate<Object>("Pierce", 14);
        cands[14] = new Candidate<Object>("Buchanan", 15);
        cands[15] = new Candidate<Object>("Lincoln", 16);
        cands[16] = new Candidate<Object>("Johnson", 17);
        cands[17] = new Candidate<Object>("Grant", 18);
        cands[18] = new Candidate<Object>("Hayes", 19);
        cands[19] = new Candidate<Object>("Garfield", 20);
        cands[20] = new Candidate<Object>("Arthur", 21);
        cands[21] = new Candidate<Object>("Cleveland", 22);
        cands[22] = new Candidate<Object>("McKinely", 23);
        cands[23] = new Candidate<Object>("Roosevely", 24);

        for(Candidate cand : cands){
            elect.addCandidate(cand);

        }

        try {
            readFile = new Scanner(file);

            elect.countVotes(readFile, cands[0]);

        } catch (FileNotFoundException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }

        System.out.println(elect.toString());
    }

}

The output from running the program:

Candidate: Washingtion
ID: 1
Votes: 1669

Candidate: Adams
ID: 2
Votes: 0

Candidate: Jefferson
ID: 3
Votes: 0

Candidate: Madison
ID: 4
Votes: 0

Candidate: Monroe
ID: 5
Votes: 0

Candidate: Quincy Adams
ID: 6
Votes: 0

Candidate: Jackson
ID: 7
Votes: 0

Candidate: Van Buren
ID: 8
Votes: 0

Candidate: Harrision
ID: 9
Votes: 0

Candidate: Tyler
ID: 10
Votes: 0

Candidate: Polk
ID: 11
Votes: 0

Candidate: Taylor
ID: 12
Votes: 0

Candidate: Fillmore
ID: 13
Votes: 0

Candidate: Pierce
ID: 14
Votes: 0

Candidate: Buchanan
ID: 15
Votes: 0

Candidate: Lincoln
ID: 16
Votes: 0

Candidate: Johnson
ID: 17
Votes: 0

Candidate: Grant
ID: 18
Votes: 0

Candidate: Hayes
ID: 19
Votes: 0

Candidate: Garfield
ID: 20
Votes: 0

Candidate: Arthur
ID: 21
Votes: 0

Candidate: Cleveland
ID: 22
Votes: 0

Candidate: McKinely
ID: 23
Votes: 0

Candidate: Roosevely
ID: 24
Votes: 0

EDIT:

Thank you Adriaan for your advice and time.

I was able to fix the problem by implementing another ArrayList to hold the votes read from the txt file. Furthermore, I now understand that using a Map as the data structure to hold the Candidate objects would be the better option.

The reason I decided to stay with my implementation is because I now have to implement a priority queue and I'm not sure how to use a Map with a priority queue. Otherwise, thank you again for the knowledge you have passed my way. Posting the updated code... Please feel free to comment on the updated solution.

Updated Candidate Class:

package problem_one;

public class Candidate {

    private String name;
    private int id;
    private int votes;

    public Candidate(String name, int id) {
        this.name = name;
        this.id = id;
        votes = 0;
    }

    public String getName() {
        return name;
    }

    public int getId() {
        return id;
    }

    public void addVote() {
        votes++;
    }

    public int getVotes() {
        return votes;
    }

    public String toString() {
        StringBuilder str = new StringBuilder();

        str.append("\nCandidate: " + getName());
        str.append("\nID: " + getId());
        str.append("\nVotes: " + getVotes() + "\n");

        return str.toString();
    }
}

The Updated Election Class:

package problem_one;

import java.util.ArrayList;
import java.util.List;
import java.util.Scanner;

public class Election {

    private List<Candidate> ballot = new ArrayList<Candidate>();
    private List<Integer> votes = new ArrayList<Integer>();

    public void addCandidate(Candidate cand) {
        ballot.add(cand);
    }

    public void readVotes(Scanner readFile) {
        int vote;

        while (readFile.hasNextInt()) {
            vote = readFile.nextInt();
            votes.add(vote);
        }
    }

    public void countVotes(Candidate cand) {
        for (int i = 1; i < votes.size(); i++) {
            if (votes.get(i).equals(cand.getId())) {
                cand.addVote();
            }
        }
    }

    public String toString() {
        StringBuilder str = new StringBuilder();

        for (Candidate cand : ballot) {
            str.append(cand);
        }
        return str.toString();
    }
}

The output:

Candidate: Washington
ID: 1
Votes: 1669

Candidate: Adams
ID: 2
Votes: 1495

Candidate: Jefferson
ID: 3
Votes: 103

Candidate: Madison
ID: 4
Votes: 32

Candidate: Monroe
ID: 5
Votes: 1474

Candidate: Quincy Adams
ID: 6
Votes: 1653

Candidate: Jackson
ID: 7
Votes: 44

Candidate: Van Buren
ID: 8
Votes: 2730

Candidate: Harrision
ID: 9
Votes: 112

Candidate: Tyler
ID: 10
Votes: 808

Candidate: Polk
ID: 11
Votes: 327

Candidate: Taylor
ID: 12
Votes: 33

Candidate: Fillmore
ID: 13
Votes: 1037

Candidate: Pierce
ID: 14
Votes: 41

Candidate: Buchanan
ID: 15
Votes: 313

Candidate: Lincoln
ID: 16
Votes: 1438

Candidate: Johnson
ID: 17
Votes: 428

Candidate: Grant
ID: 18
Votes: 1346

Candidate: Hayes
ID: 19
Votes: 1512

Candidate: Garfield
ID: 20
Votes: 1171

Candidate: Arthur
ID: 21
Votes: 539

Candidate: Cleveland
ID: 22
Votes: 132

Candidate: McKinely
ID: 23
Votes: 304

Candidate: Roosevelt
ID: 24
Votes: 33

EDIT FINAL SOLUTION:

Here is the final solution that I turned in... I didn't get to rework the code completely due to having other assignments to finish.

The comparator had to be implemented inside the for loop located in the Election class method startElectionProblemTwo. This was the only way I could get the comparator to produce the expected results. This happened because I didn't plan for changing the code down the road. Which is something I should have done from the beginning.

Also I wanted to read in the Candidates from a file but didn't get to implement this before the due date.

The comparator is used when the votes are equal. When the votes are equal we then check the number of consecutive terms served. Less terms served win, if the terms served are equal then we compare by name.

Candidate class:

package problem_one;

public class Candidate {

    private String name;
    private int id;
    private int votes;
    private int terms;

    public Candidate(String name, int id, int terms){
        this.name = name;
        this.id = id;
        this.terms = terms;
        votes = 0;
    }

    public Candidate(){

    }

    public String getName() {
        return name;
    }

    public int getId() {
        return id;
    }

    public void addVote() {
        votes++;
    }

    public int getVotes() {
        return votes;
    }

    public int getTerms() {
        return terms;
    }

    public String toString() {
        StringBuilder str = new StringBuilder();

        str.append("Candidate: " + getName());
        str.append("\nID: " + getId());
        str.append("\nTerms: " + getTerms());
        str.append("\nVotes: " + getVotes() + "\n");

        return str.toString();
    }
}

Election class:

package problem_one;

import java.io.File;
import java.io.FileNotFoundException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Scanner;

import problem_two.ElectionTieBreaker;

public class Election extends Candidate{

    // Priority Queue using a max heap.
    private List<Candidate> ballotPq;

    // Stores the votes read from the *votes* file.
    private List<Integer> votes = new ArrayList<Integer>();

    public Election(String name, int id, int terms){
        super(name, id, terms);
    }

    public Election(){
        ballotPq = new ArrayList<Candidate>();
    }

    // Change to PQ offer method.
    public void offer(Candidate cand) {
        countVotes(cand);
        ballotPq.add(cand);

        int child = ballotPq.size() - 1;
        int parent = ((child - 1) / 2);

        while (parent >= 0 && ballotPq.get(parent).getVotes() < ballotPq.get(child).getVotes()) {
            swap(parent, child);
            child = parent;
            parent = ((child - 1) / 2);
        }
    }

    // Modified Priority Queue method poll.
    private Candidate poll() {

        // Return null if the ArrayList is empty.
        if (isEmpty())
            return null;

        // Sets cand to the Candidate object at index 0.
        Candidate cand = ballotPq.get(0);

        // Check if there is only one Candidate object.
        if (ballotPq.size() == 1) {
            ballotPq.remove(0);
            return cand;
        }

        // Move the last element to the head of the Priority Queue.
        ballotPq.set(0, ballotPq.remove(ballotPq.size() - 1));
        int parent = 0;

        while (true) {

            // Finds the parents left-child, i.e. Parents left child is at 2i + 1.
            int leftChild = ((2 * parent) + 1);

            // Break out of loop.
            if (leftChild >= ballotPq.size()) {
                break;
            }

            // Finds the parents right-child, i.e. Parents right child is at 2i + 2.
            int rightChild = (leftChild + 1);

            // Assert that the leftChild is the min child 
            int minChild = leftChild;

            // If true sets the min child
            if (rightChild < ballotPq.size() && ballotPq.get(leftChild).getVotes() < ballotPq.get(rightChild).getVotes()) {
                minChild = rightChild;

            }
            //  Swaps the parent and min child if true
            if (ballotPq.get(parent).getVotes() < ballotPq.get(minChild).getVotes()) {
                swap(parent, minChild);
                parent = minChild;

            } else {break;}
        }
        return cand;
    }

    // Checks if the ArrayList is empty.
    private boolean isEmpty() {
        return ballotPq.size() == 0;
    }

    // Swaps the parent and child at the specified indices.
    private void swap(int i, int j) {
        Candidate one = ballotPq.get(i);
        Candidate two = ballotPq.get(j);

        ballotPq.set(i, two);
        ballotPq.set(j, one);
    }

    // Reads the votes from the *votes* txt file.
    private void readVotes() {
        File file = new File("votes");
        Scanner readFile;

        try {
            readFile = new Scanner(file);

            while (readFile.hasNextInt()) {
                int vote = readFile.nextInt();
                votes.add(vote);
            }

        } catch (FileNotFoundException e) {
            e.printStackTrace();
        }
    }

    // Counts the total votes casted for each Candidate.
    private void countVotes(Candidate cand) {
        for (int i = 0; i < votes.size(); i++) {
            if (votes.get(i).equals(cand.getId())) {
                cand.addVote();
            }
        }
    }

    // Starts the election for problem one
    public void startElectionProblemOne() {
        readVotes();

        offer(new Candidate("Washington", 1, 3));
        offer(new Candidate("Adams", 2, 3));
        offer(new Candidate("Jefferson", 3, 2));
        offer(new Candidate("Madison", 4, 2));
        offer(new Candidate("Monroe", 5, 2));
        offer(new Candidate("Quincy Adams", 6, 1));
        offer(new Candidate("Jackson", 7, 1));
        offer(new Candidate("Van Buren", 8, 3));
        offer(new Candidate("Harrision", 9, 1));
        offer(new Candidate("Tyler", 10, 2));
        offer(new Candidate("Polk", 11, 2));
        offer(new Candidate("Taylor", 12, 3));
        offer(new Candidate("Fillmore", 13, 2));
        offer(new Candidate("Pierce", 14, 2));
        offer(new Candidate("Buchanan", 15, 1));
        offer(new Candidate("Lincoln", 16, 1));
        offer(new Candidate("Johnson", 17, 2));
        offer(new Candidate("Grant", 18, 3));
        offer(new Candidate("Hayes", 19, 2));
        offer(new Candidate("Garfield", 20, 1));
        offer(new Candidate("Arthur", 21, 1));
        offer(new Candidate("Cleveland", 22, 4));
        offer(new Candidate("McKinely", 23, 1));
        offer(new Candidate("Roosevelt", 24, 3));

        System.out.println("Elected City Supervisor " + "\n" + poll());

        for (int i = 0; i < 4; i++) {
            System.out.println("Elected City Council " + "\n" + poll());
        }
    }

    // Starts the election for problem two
    public void startElectionProblemTwo(){
        readVotes();

        offer(new Candidate("Washington", 1, 3));
        offer(new Candidate("Adams", 2, 3));
        offer(new Candidate("Jefferson", 3, 2));
        offer(new Candidate("Madison", 4, 2));
        offer(new Candidate("Monroe", 5, 2));
        offer(new Candidate("Quincy Adams", 6, 1));
        offer(new Candidate("Jackson", 7, 1));
        offer(new Candidate("Van Buren", 8, 3));
        offer(new Candidate("Harrision", 9, 1));
        offer(new Candidate("Tyler", 10, 2));
        offer(new Candidate("Polk", 11, 2));
        offer(new Candidate("Taylor", 12, 3));
        offer(new Candidate("Fillmore", 13, 2));
        offer(new Candidate("Pierce", 14, 2));
        offer(new Candidate("Buchanan", 15, 1));
        offer(new Candidate("Lincoln", 16, 1));
        offer(new Candidate("Johnson", 17, 2));
        offer(new Candidate("Grant", 18, 3));
        offer(new Candidate("Hayes", 19, 2));
        offer(new Candidate("Garfield", 20, 1));
        offer(new Candidate("Arthur", 21, 1));
        offer(new Candidate("Cleveland", 22, 4));
        offer(new Candidate("McKinely", 23, 1));
        offer(new Candidate("Roosevelt", 24, 3));

        for (int i = 0; i < 24; i++) {
            // Comparator for candidates
            Collections.sort(ballotPq, new ElectionTieBreaker());
            System.out.println(poll());
        }
    }
}

ElectionTieBreaker class:

package problem_two;

import java.util.Comparator;

import problem_one.Candidate;

public class ElectionTieBreaker implements Comparator<Candidate> {

    @Override
    public int compare(Candidate o1, Candidate o2) {
        int flag = o2.getVotes() - o1.getVotes();

        // if the votes are the same 
        if(flag != 0){
            return flag;
        }
        if(o1.getTerms() - o2.getTerms() == 0){
            flag = o1.getName().compareTo(o2.getName());
        }else{
            flag = o1.getTerms() - o2.getTerms();
        }
        return flag;
    }
}

ProblemOneClient:

package pa2_clients;

import problem_one.Election;

public class ProblemOneClient {

    public static void main(String[] args) {
        Election election = new Election();
        election.startElectionProblemOne();
    }

}

ProblemTwoClient:

package pa2_clients;

import problem_one.Election;

public class ProblemTwoClient {

    public static void main(String[] args) {
        Election elect = new Election();
        elect.startElectionProblemTwo();
    }
}

3 Answers3

2

Normally you should declare fields as private (not protected or default).

Also declare fields by their interface, not their implementation, so:

private List<Candidate> ballot = new ArrayList<Candidate>();

You have a typo in the name of your first and ninth candidates.

Your main class has a non-standard name, change it for example to ProblemOneClient.

You have added generic class types to Candidate and Election which you are not using and don't need - remove them (the generic types, not the classes). This will also get rid of the SuppressWarnings annotation.

In your main method you are first creating an array of Candidates and then iterating over them to put them in an ArrayList. You don't need the array.

How about using a java.util.Map? You could store a candidate in it like this:

Map<Integer, Candidate> candidates = new HashMap<Integer, Candidate>();
Candidate candidate = new Candidate<Object>("Van Buren", 8);
candidates.put(candidate.getId(), candidate);

And then find the candidate by id like this:

Integer vote = 8;
Candidate candidate = candidates.get(vote);

EDIT: some more hints

I propose changing the API of Candidate:

public void addVote() {
    votes++;
}

public int getVotes() {
    return votes;
}

Inc-ing the number of votes is more convenient (and avoids fraud).

In your Election.toString() you should use StringBuilder instead of String concatenation to avoid the memory usage caused by all the intermediate Strings being interned in the String pool.

You could consider adding the candidates to a List there and sort them descending by their number of votes:

    List<Candidate> rank = new ArrayList<Candidate>(ballot.values());
    Collections.sort(rank, new Comparator<Candidate>() {
        @Override
        public int compare(Candidate o1, Candidate o2) {
            return o2.getVotes() - o1.getVotes();
        }
    });

Of course this will get done each time toString() is called so maybe a separate method would be better (toString() is normally not supposed to do a lot of work). For example you could move this into the method String Election.reportResults().

Your main method could look like this, for improved readability:

public static void main(String[] args) {
    Election election = new Election();

    addCandidates(election);

    countVotesFromFile(election, new File("votes"));

    System.out.println(election.reportResults());
}

Of course you still need to implement the methods addCandidates and countVotesFromFile.

EDIT: You are currently passing the Scanner to your Election. This makes Election aware of (and thus dependent on) the use of Scanner. It is better design to hide this implementation detail in a single class and not share it (in this case between ProblemOneClient and Election).

You also have a private List<Integer> votes in Election. I think that is not elegant; there is no need for Election to hold on to the complete list of votes.

One way of solving is to leave the iteration over votes in the ProblemOneClient and call Election.addVote(Integer) for each vote. I like this because it is simple (it is suggested by the main method I provided above).

An alternative approach is to write an implementation of Iterator<Integer> which you pass as argument to Election.addVotes(Iterator<Integer>). For a discussion of this exact issue, see here.

I made a comment about using StringBuilder your Election.toString() implementation and I noticed you also applied StringBuilder in Candidate.toString(). However in that case there is no iteration going on so you can simply format the result directly like so:

public String toString() {
    return String.format("Candidate: %s, ID: %s, Votes: %s%n", name, id, votes);
}

I put the output on a single line to make it more compact. The %s placeholder is replaced by the String representation of the matching argument. %n inserts a platform-specific newline.

EDIT: unit testing

If you are interested I have an additional assignment for you, which is to write a unit test for the Election class. You can do that using the JUnit framework.

If you are using Maven to build your project, add this dependency to your pom.xml:

<dependency>
    <groupId>junit</groupId>
    <artifactId>junit</artifactId>
    <version>4.11</version>
    <scope>test</scope>
</dependency>

If not, add this jar file to your classpath.

A unit test typically resides in a separate source directory src/test/java/. You can run it using Maven (e.g. mvn test on the command line) or within your IDE (e.g. 'run as test').

Here is a partially implemented unit test for Election:

import static org.junit.Assert.*;
// and some other imports 

public class ElectionTest {

    private Election instance;

    @Before
    public setUp() {
        instance = new Election();
    }

    @Test
    public void testAddCandidate() {
        // SETUP
        Candidate candidate = new Candidate("test", 42);

        // CALL
        instance.addCandidate(candidate);

        // VERIFY
        assertTrue(instance.reportResults().contains(candidate.toString()));
    }

    @Test
    public void testReadVotes() {
        fail("Not implemented yet");
    }

    @Test
    public void testCountVotes() {
        fail("Not implemented yet");
    }

    @Test
    public void testToString() {
        fail("Not implemented yet");
    }

    @Test
    public void testReadVotes() {
        fail("Not implemented yet");
    }
}

It is considered good practice to write unit tests for all your classes although some are more suitable for it than others. For example the ProblemOneClient is currently not easily unit-tested because it accesses a file directly. The Candidate class can easily be tested but does not contain much interesting behavior to test (only the toString method actually). Such a class is often called a model class. I personally don't prefer to write unit tests for model classes; they should be covered as part of other unit tests (they are often passed along as arguments, as also in this case).

Writing tests forces you to consider your design, easily testable classes are usually also more elegant.

Community
  • 1
  • 1
Adriaan Koster
  • 15,870
  • 5
  • 45
  • 60
  • Thank you for pointing the non-standard name used for the main class and the typos of the Candidates names. I'm going to implement a Map like you said and see if that leads me to a solution. Thank you for your time. – Robert Edstrom Mar 09 '15 at 21:30
  • Adriaan your awesome, thank you very much for going into more detail but at the same time leaving the work up to me. Sorry I can't up-vote yet... Working on this implementation now and will post the result for good measure. – Robert Edstrom Mar 09 '15 at 22:11
  • @RobertEdstrom I added a comment about unit testing. This is a very good skill for any developer. If you are interested, let me know. Also, could you post your latest main class (ProblemOneClient)? I am curious... – Adriaan Koster Mar 11 '15 at 22:04
  • @Adriann I am definitley interested. I had no idea what a unit test was until just now. At the moment im finishing off my script for Unix Programming and my Assemebly project. I'll research for addtional information about unit testing. The ProblemOne main class will be posted tonight, I am still using a Candidate array atm because I was to lazy to change it to reading a file of Candidates. I also was able to get the votes to be counted before adding them to the arraylist in the Election class. This enables me to implement the priority queue easily. – Robert Edstrom Mar 12 '15 at 23:45
  • @Adriann I will work on your assignment this weekend unless I hack out my other two tonight. So I will either post with the implementation of your assignment or with code following with questions pertaining to unit testing. – Robert Edstrom Mar 12 '15 at 23:49
  • @RobertEdstrom : I will be able to respond to any updates eat earliest one week from now. – Adriaan Koster Mar 13 '15 at 14:49
  • @ Adriann : Thank you again, I am finishing the ElectionTieBreaker class now. I will post the resulting code when I am finished. Btw, I wish I took your advice in the beginning because I am now having to rework a few things to implement the ElectionTieBreaker class that uses a comparator. – Robert Edstrom Mar 14 '15 at 22:21
  • @RobertEdstrom Soooo... any progress on this? – Adriaan Koster Mar 23 '15 at 14:14
  • @Adriann : Yes, I will post the solution I turned in to my professor in just a moment. Sorry for the delay. – Robert Edstrom Mar 24 '15 at 15:18
  • I haven't created the homework you assigned yet, but I haven't forgot. I have just been loaded down the rest of the semester. I will post my solution soon as I will have plenty of free time. Thank you again for you time and advice. – Robert Edstrom Apr 30 '15 at 03:33
1

Your issue is with this line in the main client.

elect.countVotes(readFile, cands[0]);

What that line is saying is that you want to apply all votes to the candidate in slot 0.

Given the intent, the method signature for countVotes() may need to change.

As an aside, consider using a Map, http://docs.oracle.com/javase/7/docs/api/java/util/Map.html, to store the candidates instead of an ArrayList.

Best of luck!

chiaboy
  • 394
  • 2
  • 6
1

Comments on your final solution:

I would make the name and id fields final since they should never change. You don't have to set votes to 0, this is the default for int fields. You can probably remove the default constructor, Election should definitely not extend Candidate! You could use a separate constructor for the final fields and a full constructor for convenience, but that's not really necessary. It just demonstrates you know about constructor chaining (-:

private final int id;
private final String name;

private int votes;
private int terms;

 public Candidate(int id, String name) {
    this.id = id;
    this.name = name;
}

public Candidate(int id, String name, int terms) {
    this(id, name);
    this.terms = terms;
}

As I mentioned the toString method can be replaced with:

public String toString() {
    return String.format("Candidate: %s%nID: %s%nTerms: %s%nVotes: %s%n", name, id, terms, votes);
}

I'm afraid the Election class has become a bit of a mess.

Why on earth does Election extend Candidate?

You were apparently asked to implement a priority queue with a max heap implementation. It would have been better to create the PQ implementation in a separate class and have the Election class use it, instead of merging the PQ into Election. BTW this PQ implementation is a very good class to write a unit test for.

The methods startElectionProblemOne and startElectionProblemTwo contain the same chunk of code where candidates are created. This should be extracted into a single method called from both to avoid duplication.

Why is countVotes called every time offer() is called? Counting votes is not related to the offer operation of your PQ.

Your poll() method contains while (true). In general this is a bad idea because it will be an endless loop if your logic within does not break as expected, and also not necessary. There must be some constraint on the parent variable you can test for in the loop condition...

I think readVotes() does not belong in the Election class. It should not matter to the Election class if data is read from a file, database, service or is hard coded. In fact it should be agnostic of this (information hiding). In a good design each class has a single responsibility.

When the ElectionTieBreaker encounters two candidates with equal votes and terms it is ordering by name. I think this is wrong, in this case an error should be handled because it cannot be determined which candidate won. Simply letting the candidate with the name which comes first lexicographically is not fair!

Adriaan Koster
  • 15,870
  • 5
  • 45
  • 60
  • Adriaan, thank you again for your help and time. I am still going to implement everything you advised. This semester is over and after each semester I rewrite the code that was created each semester so I can learn how to optimize my existing code. Thank again. – Robert Edstrom Apr 30 '15 at 03:31