Beginner level. Coding in C. Having trouble with "if else".

Question

Can someone please help. Im new to programming and doing the cs50 course. The task is to scramble plain text based on a code word. The language is C and on linux.

I seem to have it all working but I am banging my head for the last 2 hours trying to fix the last error when compiling the program. The problem I am having is the bottom half of the program (after the //Cipher funtion bit)

This is the error:

viginere.c:39:5: error: expected expression
else 
^
viginere.c:44:5: error: expected expression
else
^
2 errors generated.

I cant see what I have done wrong with this.

I have messed around with the {} in several different places but I dont think that is the problem as this program is a modified version of a program i made before and that one works, with the same layout (just different with a slightly different printf) What am I missing?

Heres my code:

int main (int argc, string argv[])  
{
//change command line string to a number
int k = atoi(argv[1]);
string v = argv[1];

//check program command line was typed correctly
if (argc != 2 || k != 0)
    { 
    printf("Restart program with keyword in command line (alphabetical characters only)\n");
    return 1;
    }

//Get Plain Text
printf("The keyword is set to %s.\nType in your plain text: ", argv[1]);
string s = GetString();

//Print Cipher
printf("Your cipher text = ");

//Set variables for mudulo on keyword
int codecount = strlen(argv[1]);
int c = 0;

//Cipher function (Errors are in this part)
for (int i = 0; i < strlen(s); i++)
    {
    //Cipher uppercase
    if (isupper(s[i]))
    printf("%c", (s[i] + v[c%codecount] - 65)%26 + 65);
    c++;   
    //Else Cipher lowercase
    else 
    if (islower(s[i]))
    printf("%c", (s[i] + v[c%codecount] - 97)%26 + 97);
    c++;
    //Print all else as normal
    else
    printf("%c", s[i]);
    }
printf("\n");    
}

You need braces around multiple statements after an `if` for the `else` to be valid. Also, Vigenère probably turns in his grave when his name is misspelled. — Jonathan Leffler, Aug 22 '15 at 00:23
You're missing brackets for if+else statements in your for loop — David Zech, Aug 22 '15 at 00:23
Thanks a lot guys, I managed to get this working as a result. Also changed the file name to Vigenere.... — Vince, Aug 22 '15 at 15:42

score 5 · Answer 1 · answered Aug 22 '15 at 00:24

If an if or else block has more than one statement you need to put curly braces around them. To be safe, many programmers will include braces all the time.

//Cipher uppercase
if (isupper(s[i])) {
    printf("%c", (s[i] + v[c%codecount] - 65)%26 + 65);
    c++;   
}
//Else Cipher lowercase
else if (islower(s[i])) {
    printf("%c", (s[i] + v[c%codecount] - 97)%26 + 97);
    c++;
}
//Print all else as normal
else {
    printf("%c", s[i]);
}

score 2 · Answer 2 · edited May 23 '17 at 12:14

2

You're missing the { } around the body of your if. If you don't put braces around the body, it's just the next statement. Since your else is not immediately after that statement, it's not recognized as matching up with the if.

if (isupper(s[i])) {
    printf("%c", (s[i] + v[c%codecount] - 65)%26 + 65);
    c++;
}
//Else Cipher lowercase
else if (islower(s[i])) {
    printf("%c", (s[i] + v[c%codecount] - 97)%26 + 97);
    c++;
}
//Print all else as normal
else {
    printf("%c", s[i]);
}

Even if you only have one statement in the body, I recommend you always put braces around it. See Why is it considered a bad practice to omit curly braces?

edited May 23 '17 at 12:14

Community

1
1

answered Aug 22 '15 at 00:24

Barmar

741,623
53
500
612

"Always braces" is a matter of opinion; I, for one, don't agree. But then I don't agree with the OTBS either. – Jonathan Leffler Aug 22 '15 at 00:26
@JonathanLeffler If you're so smart that you always remember to add the braces when you add the second statement to the body, all power to you. Most people make mistakes, and always bracing avoids it. – Barmar Aug 22 '15 at 00:28
@JonathanLeffler I've changed my answer to indicate that it's my opinion, not an absolute. Happy? – Barmar Aug 22 '15 at 00:28
OK. I went to the question and the accepted answers don't entirely support the question's title. I don't recall when I last got bitten by forgetting braces. Using Allmann coding style means it really isn't a problem. – Jonathan Leffler Aug 22 '15 at 00:30

Jonathan Leffler · Answer 3 · 2015-08-22T17:17:57.100

You need braces around multiple statements after an if for the else to be valid:

int len = strlen(s);
for (int i = 0; i < len; i++)
{
    //Cipher uppercase
    if (isupper(s[i]))
    {
        printf("%c", (s[i] + v[c%codecount] - 65)%26 + 65);
        c++;
    }
    //Else Cipher lowercase
    else if (islower(s[i]))
    {
        printf("%c", (s[i] + v[c%codecount] - 97)%26 + 97);
        c++;
    }
    //Print all else as normal
    else
        printf("%c", s[i]);
    printf("\n");
}

There are those who like to use braces every time. I'm of the view it isn't necessary, but it is harmless and can help you, especially perhaps while you're learning.

Alternatively, in this variant of the code, you could avoid separate statements by using the increment in the printf() calls:

int len = strlen(s);
for (int i = 0; i < len; i++)
{
    if (isupper(s[i]))         // Cipher uppercase
        printf("%c", (s[i] + v[c++ % codecount] - 'A') % 26 + 'A');
    else if (islower(s[i]))    // Cipher lowercase
        printf("%c", (s[i] + v[c++ % codecount] - 'a') % 26 + 'a');
    else                       // Non-alphabetic
        printf("%c", s[i]);
}
printf("\n");

This version also uses 'A' in place of 65 and 'a' in place of 97; it makes it easier for people to understand. This technique of compression won't always work, and should be used with caution even when it does work, but it can be used in code like this. It also only prints a newline at the end of the string (or if there are newlines within the string), rather than placing each character on its own line.

Both variations shown also avoid using strlen(s) in the loop condition; it is bad for performance, ultimately, though you would need to be encrypting long messages before it would become measurable. One could also argue that the code should use putchar(char_value) instead of printf("%c", char_value). However, this is unlikely to have a measurable benefit on the code for the data involved in a student exercise — but both changes could be significant if you were working on megabytes of data repeatedly.

score 2 · Answer 4 · answered Aug 22 '15 at 00:26

The trouble is that you have too many statements between your if and your else!

To use if...else you have to create a block or give a single statement. Really, that works out to be the same thing. In other words:

if(x==1) do_this();
else do_that();

This will work. But what is far more typical (and what you want) is to create a "Block":

if(x==1) {
  do_this();
  and_that();
} else { do_some_other_things(); }

Hope that helps!

score 1 · Accepted Answer · edited May 23 '17 at 11:44

Use curly brackets...

When there is more than one line to be executed as a result of an if(condition), each must be encapsulated with curly brackets.

if(condition)
{
    statement1();
    statement2();//without brackets this would not be executed
}

In the same way, if there are multiple lines that should be executed if and only if the if(condition) is false, they must also be surrounded with curly brackets starting after the else keyword.

...
else
{
    statement3();
    statement4();//without brackets, this would not be executed
}

There are different opinions on if curlys should be used for only single lines after if and else, but readability, maintainability and reliability are reasons enough to use curly brackets religiously.

Beginner level. Coding in C. Having trouble with "if else".

5 Answers5