cs50 scrabble game: always prints same answer, no matter what typed in

Question

i need to program a scrabble game for the cs50 course. right now i have this code but the problem is at first it always printed "Tie!" and now it always prints out "Player 2 wins!" no matter what i type in. i think my mistake is in for loop where u assign the letters to the number but i dont really know how i can fix it. does anyone may have an idea how?

// Points assigned to each letter of the alphabet
int POINTS[] = {1, 3, 3, 2, 1, 4, 2, 4, 1, 8, 5, 1, 3, 1, 1, 3, 10, 1, 1, 1, 1, 4, 4, 8, 4, 10};

int compute_score(string word1);
int compute_scores(string word2);

int main(void)
{
    // Get input words from both players
    string word1 = get_string("Player 1: ");
    string word2 = get_string("Player 2: ");

    // Score both words
    int score1 = (int) compute_score;
    int score2 = (int) compute_scores;

    // Print the winner
    if (score1 == score2)
    {
        printf("Tie! \n");
    }
    if (score1 < score2)
    {
        printf("Player 2 wins! \n");
    }
    if (score1 > score2)
    {
        printf("Player 1 wins! \n");
    }
}

int compute_score(string word1)
{
    // Compute and return score for string
    int compute_score = 0;
    int numb;
    for (int i = 0, n = strlen(word1); i < n; i++)
    {
        if(isupper(word1[i]))
        {
            numb = word1[i] - 65;
            numb = POINTS[numb];
        }
        if(islower(word1[i]))
        {
            numb = word1[i] - 97;
            numb = POINTS[numb];
        }
        else
        {
             numb = 0;
        }
    }
    compute_score = numb;
    return compute_score;
}

int compute_scores(string word2)
{
    // Compute and return score for string
    int compute_scores = 0;
    int numb;
    for (int i = 0, n = strlen(word2); i < n; i++)
    {
        if(isupper(word2[i]))
        {
            numb = word2[i] - 65;
            numb = POINTS[numb];
        }
        if(islower(word2[i]))
        {
            numb = word2[i] - 97;
            numb = POINTS[numb];
        }
        else
        {
            numb = 0;
        }
    }
    compute_scores = numb;
    return compute_scores;
}

Answer 1

When you are a beginner, there exists almost no situation where you need to use a cast, ever. Or in the rare case where you need one, you need to know what you are doing.

int score1 = (int) compute_score; converts the unsigned address of the function itself to a signed int where it might not fit. That's nonsense. You need to call the function and store the result in an int. No casting anywhere.

I'd recommend reading about functions in your favourite C book. I also recommend dropping CS-50 since it's a bad class teaching bad practices. It has a poor reputation among C programmers.

Answer 2

This

int score1 = (int) compute_score;
int score2 = (int) compute_scores;

is not how you do function calls in C. What you want is this:

int score1 = compute_score(word1);
int score2 = compute_scores(word2);

But wait... It seems the functions compute_score and compute_scores are identical. If so... you don't need two functions. Simply use the same function for both calculations

int score1 = compute_score(word1);
int score2 = compute_score(word2);

and delete the other function.

However, your current function seems wrong...

int compute_score(string word1)
{
    // Compute and return score for string
    int compute_score = 0;
    int numb;

    // Why do you have a loop here?
    // Inside the loop you keep overwriting numb so it is
    // only the last iteration that matters.
    // In other words... the result only depends on the
    // last character in the string.
    // Did you intend to do something like:
    //    numb = numb + ....
    for (int i = 0, n = strlen(word1); i < n; i++)
    {
        if(isupper(word1[i]))
        {
            numb = word1[i] - 65;
            numb = POINTS[numb];
        }
        if(islower(word1[i]))   <----- Missing else ??
        {
            numb = word1[i] - 97;
            numb = POINTS[numb];
        }
        else
        {
             numb = 0;
        }
    }
    compute_score = numb;  \
    return compute_score;   \---> These lines can simply be: return numb;
}

Maybe this is what you want:

int compute_score(string word)
{
    // Compute and return score for string
    int numb = 0;

    for (int i = 0, n = strlen(word); i < n; i++)
    {
        if(isupper(word[i]))
        {
            numb += POINTS[word[i] - 'A'];
        }
        else if(islower(word[i]))
        {
            numb += POINTS[word[i] - 'a'];
        }
    }
    return numb;
}