pset3, tideman, cs50 2020

Question

I worked with pset3 and I think there are nothing wrong with my code, but check 50 says it has four problems on my code.

it raises four problems,

1. :( add_pairs generates correct pair count when no ties add_pairs function did not produce 3 pairs

2. :( add_pairs generates correct pair count when ties exist add_pairs function did not produce 2 pairs

3. :( lock_pairs skips middle pair if it creates a cycle lock_pairs did not correctly lock all non-cyclical pairs

4. :( print_winner prints winner of election when some pairs are tied print_winner did not print winner of election

and here is my code.

Can you explain why my code is not correct?

All examples which I can imagine could pass the test with this code...

#include <cs50.h>
#include <stdio.h>
#include <string.h>

// Max number of candidates
#define MAX 9

// preferences[i][j] is number of voters who prefer i over j
int preferences[MAX][MAX];

// locked[i][j] means i is locked in over j
bool locked[MAX][MAX];

// Each pair has a winner, loser
typedef struct
{
    int winner;
    int loser;
}
pair;

// Array of candidates
string candidates[MAX];
pair pairs[MAX * (MAX - 1) / 2];

int pair_count;
int candidate_count;

// Function prototypes
bool vote(int rank, string name, int ranks[]);
void record_preferences(int ranks[]);
void add_pairs(void);
void sort_pairs(void);
void lock_pairs(void);
void print_winner(void);

int main(int argc, string argv[])
{
    // Check for invalid usage
    if (argc < 2)
    {
        printf("Usage: tideman [candidate ...]\n");
        return 1;
    }

    // Populate array of candidates
    candidate_count = argc - 1;
    if (candidate_count > MAX)
    {
        printf("Maximum number of candidates is %i\n", MAX);
        return 2;
    }
    for (int i = 0; i < candidate_count; i++)
    {
        candidates[i] = argv[i + 1];
    }

    // Clear graph of locked in pairs
    for (int i = 0; i < candidate_count; i++)
    {
        for (int j = 0; j < candidate_count; j++)
        {
            locked[i][j] = false;
        }
    }

    pair_count = 0;
    int voter_count = get_int("Number of voters: ");

    // Query for votes
    for (int i = 0; i < voter_count; i++)
    {
        // ranks[i] is voter's ith preference
        int ranks[candidate_count];

        // Query for each rank
        for (int j = 0; j < candidate_count; j++)
        {
            string name = get_string("Rank %i: ", j + 1);

            if (!vote(j, name, ranks))
            {
                printf("Invalid vote.\n");
                return 3;
            }
        }

        record_preferences(ranks);
        printf("\n");
    }

    add_pairs();
    sort_pairs();
    lock_pairs();
    print_winner();
    return 0;
}

// Update ranks given a new vote
bool vote(int rank, string name, int ranks[])
{
    for (int i = 0; i < candidate_count; i++)
    {
        if (strcmp(candidates[i], name) == 0)
            {
            ranks[rank] = i;
            return true;
            }
    }
return false;
}

// Update preferences given one voter's ranks
void record_preferences(int ranks[])
{
    for (int i = 0; i < candidate_count; i++) {
        for (int j = 0; j < candidate_count; j++){
            if(i < j){
                preferences[ranks[i]][ranks[j]] += 1;
            }
        }
    }
    return;
}

// Record pairs of candidates where one is preferred over the other
void add_pairs(void)
{
    for (int i = 0; i < MAX; i++){
        for (int j = 0; j < MAX; j++){
            if (preferences[i][j] > preferences[j][i]){
                for (int k = 0; k < MAX * (MAX - 1) / 2; k++){
                    if (pairs[k].winner == 0 & pairs[k].loser == 0){
                        pairs[k].winner = i;
                        pairs[k].loser = j;
                        break;
                    }
                }
            }
        }
    }
    return;
}

// Sort pairs in decreasing order by strength of victory 여기까진 됐음
void sort_pairs(void)
{
    for (int j = 0; j <  MAX * (MAX - 1) / 2; j++){
        for (int i = 0; i <  MAX * (MAX - 1) / 2; i++){
            if (preferences[pairs[i].winner][pairs[i].loser] < preferences[pairs[i+1].winner][pairs[i+1].loser]) {
                pair temp = pairs[i];
                pairs[i] = pairs[i+1];
                pairs[i+1] = temp;
            }
        }
    }
    return;
}

// Lock pairs into the candidate graph in order, without creating cycles
void lock_pairs(void)
{
int check = 0;
int array[candidate_count];
int vertical[candidate_count];
int vcheck = 0;

    for (int i = 0; i <  MAX * (MAX - 1) / 2; i++){
        if (pairs[i].winner == 0 & pairs[i].loser == 0){
            return;
        }
        else {
            locked[pairs[i].winner][pairs[i].loser] = true;
            check = 0;

            for (int m = 0; m < candidate_count; m++){
                array[m] = 0;
                vertical[m] = 0;
            }

            for (int k = 0; k < candidate_count; k++){
                for (int j = 0; j < MAX; j++){
                    if(locked[k][j] == false){
                        array[k] += 1;
                    }
                    if (locked[j][k] == false) {
                        vertical[k] += 1;
                    }
                }
            }

            for (int o = 0; o < candidate_count; o++){
                if (array[o] == MAX) {
                    check += 1;
                }
                if (vertical[o] == MAX){
                    vcheck += 1;
                }
            }

            if (check == 0 | vcheck == 0) {
                locked[pairs[i].winner][pairs[i].loser] = false;
            }
        }
    }
    return;
}

// Print the winner of the election
void print_winner(void)
{
int array[MAX];

    for (int u = 0; u < MAX; u++){
        array[u] = 0;
    }

    for (int i = 0; i < MAX; i++){
        for (int j = 0; j < MAX; j++){
            if (locked[i][j] == true){
                array[i] += 1;
            }
        }
    }

    int max = array[0];
    int count = 0;
    int key[MAX];

    for (int i = 0; i < MAX; i++){
        key[i] = 11;
    }

    for (int k = 0; k < MAX; k++){
        if (max < array[k]){
            max = array[k];
        }
    }

    for (int m = 0; m < MAX; m++){
        if (max == array[m]){
            count += 1;
            key[m] = m;
        }
    }

    if (count > 1) {
        for (int m = 0; m < MAX; m++){
            if (key[m] != 11){
                for (int k = 0; k < MAX; k++){
                    if (locked[k][m] == true) {
                        printf("%s\n", candidates[k]);
                    }
                }
            }
        }
    }
    else {
        for (int m = 0; m < MAX; m++){
            if(key[m] != 11){
                printf("%s\n", candidates[m]);
            }
        }
    }
    return;
}

Answer 1

add_pairs should increment pair_count.

sort_pairs would access an element beyond the array length. If you have pair_count elements, you have pair_count-1 neighbour comparisons per round, one less than array elements.

Not sure how your lock_pairs is meant to work (maybe you could edit an explanation into your question?), again you are not using pair_count. For an edge from winner to loser to create a cycle, there would have to be some path (over an arbitrary number of locked edges) from the loser to the winner. Maybe "transitive closure" is a thing worth researching.

Same for your print_winner, I don't understand it, and it feels overly complicated. You should print the name of any candidate that does not lose in any locked edge. Shouldn't use MAX, but probably candidate_count, and would be much shorter.