-3

So i recently started c language with no prior knowledge of coding or computer science. Wrote this piece of code to find value of a word using scrabble points as below: 1:AEILNORSTU 2:DG 3:BCMP 4:FHVWY 5:K 8:JX 10:QZ.

# include <stdio.h>
# include <ctype.h>
# include <conio.h>
int main (void)
{
  int n=0;
  char ch;
  clrscr();
  printf("Enter the SCRABBLE word\n");
  ch = getchar();
  while(ch!='\n')
  {
    toupper(ch);
    if(ch =='A'||'E'||'I'||'L'||'N'||'O'||'R'||'S'||'T'||'U')
      n=n+1;
    else if (ch =='D'||'G')
      n=n+2;
    else if (ch =='B'||'C'||'M'||'P')
      n=n+3;
    else if (ch =='F'||'H'||'V'||'W'||'Y')
      n=n+4;
    else if (ch =='K')
      n=n+5;
    else if (ch =='J'||'X')
      n=n+8;
    else if (ch =='Q'||'Z')
      n=n+10;

    ch = getchar();
  }
  printf("The value is %d",n);
  return 0;
}

So what happens when i run this code is that : Enter the SCRABBLE word eg: barrier The value is 7 though it should be 9 as b carries 3 points as noted above the code,a carries 1,r carriers 1,again r 1 point,i carries 1 point and the last two alphabet are one point each so thats 3+1+1+1+1+1+1=9

Nayan T
  • 1
  • 1
  • What is a Scrabble word?? You said Scrabble word, but you are giving a character as input.. plz clarify – surjit Jun 21 '17 at 11:54
  • Please learn about *indentation*. Also consider using empty lines to divide the code into blocks. It will make the code easier to read, understand and maintain. – Some programmer dude Jun 21 '17 at 11:54
  • 1
    Furthermore, the [`getchar`](http://en.cppreference.com/w/c/io/getchar) returns an `int`. It's important when you check for errors or end-of-file (when `getchar` returns `EOF`). Something you really should check for. – Some programmer dude Jun 21 '17 at 11:56
  • "I'm not getting the correct answer" isn't helpful. – David Hoelzer Jun 21 '17 at 12:13
  • What are you inputting? What are you getting out? (What is the expected output? Is case suppose to be sensitive? (This only allows uppercase words) – Tezra Jun 21 '17 at 12:50
  • Use the right type as commented, `char ch;` ==> `int ch;` and another important technique is to *read the man page* of every function you use. – Weather Vane Jun 21 '17 at 14:56
  • Welcome to Stack Overflow! It is difficult to offer solutions when the problem statement is simply, "it doesn't work". Please [edit] your question to give a more complete description of what you expected to happen and how that differs from the actual results. See [ask] for hints on what makes a good explanation. – Toby Speight Jun 21 '17 at 16:47

2 Answers2

1

An expression like ch =='D'||'G' is equal to (ch == 'D')||'G'.

In other words you first perform the sub-expression ch == 'D'. Then you do a logical or using 'G'. The result will always be true since 'G' is non-zero, and everything non-zero is true in C.

You want ch == 'D' || ch == 'G' instead, to check if ch is equal to 'D' or if ch is equal to 'G'.

This is very basic and every good beginners book would have told you so.

In the specific case of the code you show, the very first check will always be true because of this, and you will not check any other cases.

Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
  • Thanks this seems to have solved my problem.Except another one crept up. It always shows 0 if i input alphabets in lower case. If i input using caps lock the answer is right. Guess some problem in my 'toupper()' ? Thanks again – Nayan T Jun 21 '17 at 12:01
  • @NayanT The [`toupper`](http://en.cppreference.com/w/c/string/byte/toupper) function *returns* the new character. It can't modify it in place. – Some programmer dude Jun 21 '17 at 12:03
  • Ah Thanks again. Will be sure to practicing on my indentation too. ;D – Nayan T Jun 21 '17 at 12:11
-1

you forgot to get the return value of toupper. Also you don't check it (according to man 3 toupper, the function will return the same char in case of failure)

Then your conditions aren't good: if (ch == 'Q' || 'Z') is different from if (ch == 'Q' || ch == 'Z') The || means if the right or the left condition is true and in the first case (ch == 'Q') is not always true, but 'Z' is always true.

This means your first condition :

if(ch =='A'||'E'||'I'||'L'||'N'||'O'||'R'||'S'||'T'||'U')

is always true

Here are littles corrections you can apply, this might work

    char         tmp;
    ch = getchar();
    while(ch!='\n')
    {
        //Stocking in a tmp char to check return value
        tmp = ch;

        //Here
        ch = toupper(ch);
        if (ch == tmp)
        {
          //Error
          break;
        }
        {...}
        else if (ch =='Q'|| ch =='Z')
          n += 10;
        ch = getchar();
    }

Edit : Correction of return value of toupper();

drumz
  • 65
  • 7
  • Hello thanks . Yes some problem with my toupper. Could you please explain a bit about the if block containing break.is it checking for error.How? Thanks again. – Nayan T Jun 21 '17 at 12:10
  • Hello thanks . Yes some problem with my toupper. Could you please explain a bit about the if block containing break.is it checking for error.How? Thanks again. – Nayan T Jun 21 '17 at 12:12
  • As Drumz said, `toupper()` will return `'c'` if it fails. So you have to check if `ch` is equal to `'c'` to see if there was an error. – Silveris Jun 21 '17 at 12:27
  • @Drumz You have misread. `toupper` does not return `'c'`. When no conversion is made it returns the input character. – stark Jun 21 '17 at 13:06
  • @stark Okay thanks, my bad – drumz Jun 21 '17 at 13:36
  • @NayanT Break is a keyword that "break" the loop (it will stop your while loop even if the condition is true) – drumz Jun 21 '17 at 13:36
  • If `ch` already is an upper case character no conversion is done. You shouldn't treat this as an error. Calling `isalpha(ch)` might be more useful to detect errors. – Gerhardh Jun 21 '17 at 13:51
  • Or just handle in a default `else` case. – stark Jun 21 '17 at 14:23
  • Ah understood. so that is for checking error.neat trick. – Nayan T Jun 22 '17 at 02:25