0

In the code below, strings in argv never matches the user input strings.

For example, if I execute a code with line ./plurality Alice Bob, the names in argc are correctly transferred to the candidates array, but they just never match with the strings that is received later in the program. (Unnecessary part of the programs were removed, for the sake of readability.)

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

// Max number of candidates
#define MAX 9

// Candidates have name and vote count
typedef struct
{
    string name;
    int votes;
}
candidate;

// Array of candidates
candidate candidates[MAX];

// Number of candidates
int candidate_count;

// Function prototypes
bool vote(string name);
void print_winner(void);

int main(int argc, string argv[])
{
    for (int i = 0; i < candidate_count; i++)
    {
        candidates[i].name = argv[i + 1];
        printf("%s ", candidates[i].name);
        candidates[i].votes = 0;
    }
    printf("\n");

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

    // Loop over all voters
    for (int i = 0; i < voter_count; i++)
    {
        string name = get_string("Vote: ");

        // Check for invalid vote
        if (!vote(name))
        {
            printf("Invalid vote.\n");
        }
    }
}

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

This is driving me crazy, any kind of help would be appreciated.

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
Jack
  • 11
  • 3

2 Answers2

3
  1. The global variable int candidate_count in implicitly initialized to 0 so you want to assign it a value if you want something else. Consider using argc in your loop condition, you need to anyways, to safely index argv[i + 1].

  2. Use !strcmp() to compare two strings for equality.

  3. (Advise) Don't use global variables.

  4. (Not fixed) Consider using lfind()in vote().

  5. (Idea) It will not be faster till probably 1,000+ votes but you could also sort your candidate array (qsort()) and use binary search (bsearch()). Alternatively keep the array sorted as you add candidates. The insight here is that number of candidates is probably much smaller than number of votes.

  6. (Idea) Consider using a struct of arrays instead of array of structs:

struct candidates {
   string *names;
   int *votes;
}

It's a little more work to dynamically allocate the names and votes array, but it maps more directly to your problem including your search use case.

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

#define MAX 9

typedef char * string;
typedef struct {
    string name;
    int votes;
} candidate;

bool vote(size_t len, candidate candidates[len], string name) {
    for (size_t i = 0; i < len; i++)
        if (!strcmp(candidates[i].name, name)) {
            candidates[i].votes++;
            return true;
        }
    return false;
}

int main(int argc, string argv[]) {
    size_t candidate_count = argc - 1;
    // if(candidate_count > MAX) {
    //  printf("too many candidates\n");
    //  return 1;
    // }
    candidate candidates[candidate_count];
    for (size_t i = 0; i < candidate_count; i++) {
        candidates[i].name = argv[i + 1];
        printf("%s ", candidates[i].name);
        candidates[i].votes = 0;
    }
    printf("\n");
    int voter_count = get_int("Number of voters: ");
    for (int i = 0; i < voter_count; i++) {
        string name = get_string("Vote: ");
        if (!vote(candidate_count, candidates, name)) {
            printf("Invalid vote.\n");
        }
    }
}
Allan Wind
  • 23,068
  • 5
  • 28
  • 38
1

canidates[i].name == name Compares the addresses of the two strings.

Use strcmp() instead.

Anonymous
  • 66
  • 4