1

I am getting an error of "lvalue required as left operand of assignment".

It assigns to the line:

if(ch=='.' || ch=='"' || ch=='(' || ch=')' || ch=='!' || ch==',' || ch=='?' || ch==';') { etc }

I just want to check if the character I'm looking at is equal to any one of those characters. Any idea on the fix?

Thank you,

Mysticial
  • 464,885
  • 45
  • 335
  • 332
Ellea
  • 41
  • 1
  • 1
  • 4

2 Answers2

10

ch=')' should be ch==')'. The reason you are getting that particular error is because while == has higher precedence than ||, = has lower precedence, so this:

ch == '(' || ch = ')'

is parsed as

((ch == '(') || ch) = ')'

Which is trying to assign ')' to the result of (ch == '(') || ch) which is an rvalue and not assignable. This is a common mistake, and this technique is used to avoid it:

if ('.' == ch || '"' == ch || ...)

Notice that the character literals and the variable have switched places around the ==. That way, if you accidentally type

if ('.' = ch)

You'll get a compiler error.

Seth Carnegie
  • 73,875
  • 22
  • 181
  • 249
0

Your immediate problem is that you are using = (assignment) rather the == (equality):

if(ch=='.' || ch=='"' || ch=='(' || ch=')' || ...
                                   ___^____
                                   see here

What's happening is to do with the relative precedence of the various bits. Although == will bind at a higher level than ||, a == b || c == d evaluates as (a == b) || (c == d).

However, = does not bind at a higher level so a == b || c = d evaluates as ((a == b) || c) = d). With your code, that ends up with a part expression of:

('(' || ch) = ')'

In other words, you're trying to assign ')' to the non-lvalue '(' || ch.

You actually got lucky there by not parenthesising every term (which many people do) since that would make it syntactically correct but not do what you expect at runtime:

if ((ch == '.') || (ch == '"') || (ch == '(') || (ch = ')') || ...
                                        _________^^^^^^^^^^_________
                                        will assign rather than test

The usual approach to avoid that is to put the constant first but, like you, I believe that's just plain ugly:

if (('.' == ch) || ('"' == ch) || ('(' == ch) || (')' = ch) || ...
                                                  ^^^^^^^^
                                                  / urk! \

When I'm doing that sort of thing, I tend to rely on the standard library so that my code looks better and has much less chance of being incorrect:

if (strchr (".\"()!,?;", ch) != NULL) {
    // It's valid.
}

or even better:

#define VALID_PUNCTUATION ".\"()!,?;"
if (strchr (VALID_PUNCTUATION, ch) != NULL) {
    // It's valid.
}

or even better than that, put the whole segment in a function and just use something like:

if (isValidPunctuation (ch)) {
    // It's valid.
}

(you'll still have to make sure the code in the function is correct but your mainline code will be much cleaner).

paxdiablo
  • 854,327
  • 234
  • 1,573
  • 1,953