My scrabble solution for Week 2 of CS50, struggles with symbols. Cant figure out why

Question

So I wrote this code for the scrabble problem for the lab of week 2 of CS50. The jist of it is that, we need to write code to calculate the score of two words given by two players, and then declare the winner based on the points assigned to each letter in both the words. The points assigned to each letter is given in the array "POINTS".

Here's a link to the problem if u need more info: https://cs50.harvard.edu/x/2023/labs/2/

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

// 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 };

string convert_upper(string word);
int compute_score(string upper_word);
void print_winner(int score1, int score2);

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

    // Convert to uppercase
    string upper_word1 = convert_upper(word1);
    string upper_word2 = convert_upper(word2);

    // Calculate scores
    int score1 = compute_score(upper_word1);
    int score2 = compute_score(upper_word2);

    // TODO: Print the winner
    print_winner(score1, score2);
}

string convert_upper(string word)
{
    // Convert to uppercase
    int length = strlen(word);
    for (int i = 0; i < length; i++)
    {
        word[i] = toupper(word[i]);
    }
    printf("%s\n", word);
    return word;
}

int compute_score(string upper_word)
{
    // Add score for each letter
    int score = 0;
    for (int i = 0; i < strlen(upper_word); i++)
    {
        score = score + POINTS[upper_word[i]-65];
    }
    return score;
}

void print_winner(int score1, int score2)
{
    // Print result based on the scores
    if (score1 > score2)
    {
        printf("Player 1 wins \n");
    }
    else if (score2 > score1)
    {
        printf("Player 2 wins \n");
    }
    else
    {
        printf("Tie! \n");
    }
}

The code works fine if I just use normal words. But as soon as I use a word with a symbol at the end like question?, the score gets too high and it doesn't work as intended. It seems like there is some problem with how my code handles the symbols. I know that isupper() is a better way to implement this without any errors, but I want to know what went wrong here exactly.

the cs50 library is used here to get access to the get_string function which allows me to get a string from the user. — King Brain, Jul 05 '23 at 15:23
The most important thing for you to learn is [how to use a debugger](https://idownvotedbecau.se/nodebugging/). Pay particularly close attention to the value of `upper_word[i]-65` when `upper_word[i]` is a punctuation character. — Corvus, Jul 05 '23 at 15:27
i did that....and as soon as i=8, the value of score jumps to a 6 digit number — King Brain, Jul 05 '23 at 15:31
What do you expect to happen in `POINTS[upper_word[i]-65]` if you don't provide a letter? What is allowed range for the index of `POINTS`? What is `upper_word[i]-65` in your error case? — Gerhardh, Jul 05 '23 at 15:36
So the expression POINTS[upper_word[i] - 65] is supposed to subtract 65 from the ASCII position of the letter in position i. 65 is the position of A in ASCII. So this way im able to match the letters with their corresponding points in the POINTS array. But the problem comes with certain symbols like "?". — King Brain, Jul 05 '23 at 15:39
The answer is to filter out non-alpha characters from the string, which you cab do by building another string, or shuffling the alpha characters down the input string (adding a string terminator in both cases). At the same time you can convert lower case to upper. So for example `if(islpha(instr[i])) { instr[j++] = toupper(islpha(instr[i]); }` — Weather Vane, Jul 05 '23 at 15:39
Don't subtract `65` but subtract `'A'` to get the index into score array. — Weather Vane, Jul 05 '23 at 15:40
When i enter "question?" for Player1, it gives a score1 output of 1934385241. — King Brain, Jul 05 '23 at 15:40
As commented, filter out the `'?'`. Obviously, it's not an acceptable Scabble play. Another way, is to examine every character of the input string and rehject the entire entry if it isn't actually a word. — Weather Vane, Jul 05 '23 at 15:41
Ill do that but id gladly appreciate it if you could tell me why exactly a symbol like "?" is giving me such a huge score? Cuz from my point of view, since the position of the symbol doesnt exist in the array, shouldnt it just not add at all? — King Brain, Jul 05 '23 at 15:41
Because you are indexing somewhere outside the array, which is *undefined behaviour*. C doesn't have a concept of "doesn't exist", but blindly does what you tell it to do. — Weather Vane, Jul 05 '23 at 15:42
Oh so for example, if something in the word has an ASCII position of lets say 20 and i subtract 65 from it, does it end up with a negative position? And if it does end up with such a position, shouldnt the computer just ignore it cuz such a position doesnt exist? Dont positions for arrays start from 0? — King Brain, Jul 05 '23 at 15:45
No, "the computer" does not check array indexing. It does what you tell it to, which is why C is dangerous, but it means execution is quick. You must check array indexing *yourself* if there is any possibility of exceeding its bounds. — Weather Vane, Jul 05 '23 at 15:46

chqrlie · Answer 1 · 2023-07-05T15:47:26.457

In compute_score you access the score for a given tile as POINTS[upper_word[i] - 65];. If the character at offset i is not an uppercase letter, the index computed as upper_word[i] - 65 is outside the range of array POINTS so accessing this entry has undefined behavior. Note that you assume that uppercase letters are contiguous in the character set and that A is encoded as 65, which is specific to ASCII. The program would fail on a target that uses EBCDIC (but you are unlikely to find one).

You should reject a word that is not exclusively composed of letters and possibly some wildcard such as * or for the place holder.

Here is a modified version:

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

// 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
};

string convert_upper(string word);
int compute_score(string upper_word);
void print_winner(int score1, int score2);

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

    // Convert to uppercase
    string upper_word1 = convert_upper(word1);
    string upper_word2 = convert_upper(word2);

    // Calculate scores
    int score1 = compute_score(upper_word1);
    int score2 = compute_score(upper_word2);

    // TODO: Print the winner
    print_winner(score1, score2);
}

string convert_upper(string word)
{
    // Convert to uppercase
    int length = strlen(word);
    for (int i = 0; i < length; i++) {
        unsigned char cc = word[i];
        word[i] = toupper(cc);
    }
    printf("%s\n", word);
    return word;
}

int compute_score(string upper_word)
{
    // Add score for each letter
    int length = strlen(word);
    int score = 0;
    for (int i = 0; i < length; i++) {
        unsigned char cc = upper_word[i];
        if (!isupper(cc)) {
            if (cc != ' ' && cc != '*') {
                printf("invalid word: %s\n", upper_word);
                return 0;
            }
        } else {
            score = score + POINTS[cc - 'A'];
        }
    }
    return score;
}

void print_winner(int score1, int score2)
{
    // Print result based on the scores
    if (score1 > score2) {
        printf("Player 1 wins \n");
    } else
    if (score2 > score1) {
        printf("Player 2 wins \n");
    } else {
        printf("Tie! \n");
    }
}

Thanks for this info. I had one last question. How is the computer able to tell that i am subtracting 65 from the ASCII value of the letter stored at that position? Cuz when u normally try to subtract 65 from A, it doesnt work cuz A is a string. How come it works when its in an array? — King Brain, Jul 05 '23 at 15:55
@KingBrain: `'A'` is not a string, it is a character. The type for characters in C is `char`, an integer type that can be signed or unsigned depending on the platform. Using `unsigned char` ensures that the characters from the string are constrained to the range `0..UCHAR_MAX` which is required for the `toupper()` and `isupper()` functions. On systems that use ASCII (almost all of them today), `'A'` has the value `65` so you could use `65`, but it is more readable to use `'A'` to show that you are computing the offset in the alphabet from `A`, assuming all uppercase letters are consecutive. — chqrlie, Jul 05 '23 at 16:01
oh i see...so if i were to use A as char and then tried to subtract 65 from it, it should give 0 right? — King Brain, Jul 05 '23 at 16:06
More specifically, `'A' - 'A'` is `0`, and `'B' - 'A'` is `1`, etc provided they are consecutive, which they are in ASCII. You don't need to know their *actual* codes. — Weather Vane, Jul 05 '23 at 16:10

Anant · Answer 2 · 2023-08-02T12:32:59.963

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

// 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 word);

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 = compute_score(word1);
    int score2 = compute_score(word2);

    // TODO: Print the winner
    if(score1>score2)
    printf("Player 1 wins!\n");
    else if (score2>score1)
    printf("Player 2 wins\n");
    else printf("Tie!\n");
}

int compute_score(string word)
{
    // TODO: Compute and return score for string
    int sum=0;
    // loop to iterate across string
    for(int i=0;i<strlen(word);i++)
    {
        //reject all the characters that come before letters in ascii chart
        //make them 0 so that they dont add up the score and return garbage value
        if(word[i]>=1 &&word[i]<=64)
        {
            word[i]=0;
        }
        //convert all to uppercase so we dont have to check twice
        word[i]=toupper(word[i]);
        int a=word[i]-'A';
        // counter
        sum=sum+POINTS[a];

    }
     return sum;

}

// Code by anant

As you call `strlen()` in the loop itself, the loop will end as soon as you set a character to `0`. You should use `strlen` once before the loop. — Bruno, Aug 04 '23 at 14:16
yeah we can avoid this by using a if condition above which checks if length is zero and terminates the whole code by using return 1; — Anant, Aug 24 '23 at 18:19

My scrabble solution for Week 2 of CS50, struggles with symbols. Cant figure out why

2 Answers2