warning C4706: assignment within conditional expression

Samuel Cragg

I downloaded some code of the internet which is used as part of Firefox, OpenOffice etc, compiled it and it produced a few warnings. I'm not a fan of warnings so thought I'd have a look to fix it...

if (((rv) &&
( checked_prefix || (words && words[wnum]) ||
(compoundflag && TESTAFF(rv->astr, compoundflag, rv->alen)) ||
((oldwordnum == 0) && compoundbegin && TESTAFF(rv->astr, compoundbegin, rv->alen)) ||
((oldwordnum > 0) && compoundmiddle && TESTAFF(rv->astr, compoundmiddle, rv->alen))// ||
// (numdefcpd && )

// LANG\_hu section: spec. Hungarian rule
|| ((langnum == LANG\_hu) && hu\_mov\_rule && (
TESTAFF(rv->astr, 'F', rv->alen) || // XXX hardwired Hungarian dictionary codes
TESTAFF(rv->astr, 'G', rv->alen) ||
TESTAFF(rv->astr, 'H', rv->alen)
)
)
// END of LANG\_hu section
) &&
(
// test CHECKCOMPOUNDPATTERN conditions
scpd == 0 || checkcpdtable\[scpd-1\].cond == FLAG\_NULL || 
TESTAFF(rv->astr, checkcpdtable\[scpd-1\].cond, rv->alen)
)
&& ! (( checkcompoundtriple && scpd == 0 && !words && // test triple letters
(word\[i-1\]==word\[i\]) && (
((i>1) && (word\[i-1\]==word\[i-2\])) ||
((word\[i-1\]==word\[i+1\])) // may be word\[i+1\] == '\\0'
)
) ||
(
checkcompoundcase && scpd == 0 && !words && cpdcase\_check(word, i)
))
)
// LANG\_hu section: spec. Hungarian rule
|| ((!rv) && (langnum == LANG\_hu) && hu\_mov\_rule && (rv = affix\_check(st,i)) &&
(sfx && sfx->getCont() && ( // XXX hardwired Hungarian dic. codes
TESTAFF(sfx->getCont(), (unsigned short) 'x', sfx->getContLen()) ||
TESTAFF(sfx->getCont(), (unsigned short) '%', sfx->getContLen())
)
)
)
) {
	// 248 lines of code
}

In fairness, it's the first time I've seen so many comments inside an if statement! Think I'll live with the warning (i.e. disable it for that particular file.)

Monjurul Habib

This is simply ridiculous.. :zzz: :zzz:

makumazan84

33 lines long condition in this piece of code is absolutely adorable! A drunk disassembler would have done a better job generating such code :) No kidding - recently, I had to restore some source code from a compiled dll, and reflector's result was way more reasonable than this one.

oggenok64

I've never seen this code before but assume it must be TKSpell. Yes, it's a horribly long conditional, but the code layout and comments make it reasonably clear as to what's happening. I know that i'm going to offend a lot of people, but a large function is not, repeat not, in itself a sign of bad programming. If the routine does exactly one well defined thing, it doesn't matter how large it is counting lines of code. And now i hear the refactoring crowd shouting at me. OK guys, you spend your time refactoring. I spend my time producing good solid production code.

Lutoslaw

Sam Cragg wrote:

)
)
)
)

IHMO this is the best part of it.

Greetings - Jacek

jsc42

The layout is not pretty and I would have commented it a lot more thoroughly, but there is nothing wrong with a complex if statement. I know that the refactoring polizei will feel that 1000 statements are better than one but as long as the code is well laid out and well commented (which this isn't), then a single statement can be simpler to follow. The warning, incidentally, is for the final stanza in the line after the comment // LANG_hu section: spec. Hungarian rule. The compiler is guessing that rv = affix_check(st,i) should be rv == affix_check(st,i)