0

So I've been working on a lot of CS50 projects lately and I've stumbled on a roadblock when I got to Lab 2. Now, this lab was really difficult for me, but I managed to get it work... sort of. When I run it, it displays the two prompts, but no matter what I type in, it will always be a tie.

#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 word1, string word2);

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

    // Convert to uppercase
    int toupper(int word1);
    int toupper(int word2);

    // Score both words
    int score = compute_score(word1, word2);
    // TODO: Print the winner
    if (score == 1) {
        printf("Player 1 Wins!");
    }
    else if (score == 2) {
        printf("Player 2 Wins!");
    }
    else {
        printf("Tie!");
    }
    printf("\n");
}

int compute_score(string word1, string word2)
{
    int i, j, k, n1=strlen(word1), n2=strlen(word2), p1=0, p2=0;
    const int alphabetSize = 26;
    char letters[] = {'A','B','C','D','E','F','G','H','I','J','K','L','M','N','O','P','Q','R','S','T','U','V','W','X','Y','Z'};
    for (i=0; i<n1 + n2; i++) {
        for (j=0; j<alphabetSize; j++) {
            if (letters[j] == word1[i]) {
                p1 += POINTS[j];
            }
        }
        for (k=0; k<alphabetSize; k++) {
            if (letters[k] == word2[i]) {
                p2 += POINTS[k];
            }
        }

    }
    if (p1 > p2) {
        return 1;
    }
    else if (p1 < p2) {
        return 2;
    }
    else {
        return 3;
    }
}

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335

2 Answers2

2

These two lines

// Convert to uppercase
int toupper(int word1);
int toupper(int word2);

are nothing more than declarations of the same function toupper.

If you want to convert the strings to upper case when write a separate function yourself as for example

string string_toupper( string word )
{
    for ( string s = word; *s != '\0'; ++s )
    {
        *s = toupper( ( unsigned char ) *s );
    }

    return word;
}

And in main instead of the two lines shown above write

// Convert to uppercase
string_toupper( word1 );
string_toupper( word2 );

This for loop within the function compute_score

for (i=0; i<n1 + n2; i++) {

results in undefined behavior due to accessing memory outside the strings for indices in the range [0, n1 + n2)

Instead write a separate function that calculates a score for a string like

int compute_score( string word )
{
    int score = 0;

    string letters = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
    // 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};

    for ( ; *word != '\0'; ++word ) 
    {
        string p = strchr( letters, *word );
        if ( p != NULL ) score += POINTS[p - letters];
    }

    return score;
}

and in main you can write

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

// TODO: Print the winner
if ( score2 < score1 ) {
    puts("Player 1 Wins!");
}
else if ( score1 < score2 ) {
    puts("Player 2 Wins!");
}
else {
    puts("Tie!");
}

Pay attention to that instead of the typedef name string it will be much better to use types char * and const char *.

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
0

The following solution is done using the Ada programming language. This is offered to show that the language chosen to solve a problem can impact the way the solution is expressed.

-- Ada version of CS50 Lab 2

with Ada.Text_IO;             use Ada.Text_IO;
with Ada.Characters.Handling; use Ada.Characters.Handling;

procedure Main is
   subtype Upper_Case is Character range 'A' .. 'Z';
   Letters : constant array (Upper_Case) of Positive :=
     ('A' => 1, 'B' => 3, 'C' => 3, 'D' => 2, 'E' => 1, 'F' => 4, 'G' => 2,
      'H' => 4, 'I' => 1, 'J' => 8, 'K' => 5, 'L' => 1, 'M' => 3, 'N' => 1,
      'O' => 1, 'P' => 3, 'Q' => 10, 'R' => 1, 'S' => 1, 'T' => 1, 'U' => 1,
      'V' => 4, 'W' => 4, 'X' => 8, 'Y' => 4, 'Z' => 10);
   type Winner is (Player1, Player2, tie);

   function compute_score (left : String; right : String) return Winner is
      p1   : Natural := 0;
      p2   : Natural := 0;
      Temp : Character;
   begin
      for value of left loop
         Temp := To_Upper (value);
         if Temp in Upper_Case then
            p1 := p1 + Letters (Temp);
         end if;
      end loop;
      for value of right loop
         Temp := To_Upper (value);
         if Temp in Upper_Case then
            p2 := p2 + Letters (Temp);
         end if;
      end loop;
      if p1 > p2 then
         return Player1;
      elsif p2 > p1 then
         return Player2;
      else
         return tie;
      end if;
   end compute_score;

begin
   Put ("Enter two lines of text:");
   declare
      Line1  : String := Get_Line;
      Line2  : String := Get_Line;
      Result : Winner;
   begin
      Result := compute_score (Line1, Line2);
      case Result is
         when Player1 =>
            Put_Line ("Player 1 wins");
         when Player2 =>
            Put_Line ("Player 2 wins");
         when tie =>
            Put_Line ("Tie");
      end case;
   end;
end Main;

The scores are based upon only letters and not any other characters in the entered string. You must ensure that you are not trying to accumulate scores for white space, digits, or punctuation.

Jim Rogers
  • 4,822
  • 1
  • 11
  • 24