To me this is a coding horror, and to you? [modified]

[geoffs]

Certainly true. I think the real issue is whether or not the original line has any more clarity than the one I prefer coded up. IMO it does not. What part of "logical expression yielding a boolean result" is tough for any programmer to read and understand? At some point, verbosity for clarity's sake can become ridiculous.

modified on Tuesday, September 16, 2008 11:02 AM

[BillW33]

I have to agree. m_boolVar = intVar != 0; is much clearer and easier to read, IMO. Bill W

[Lost User]

In fact I just had the very same situation today. A call to a SAP-Websewrvice yielded an integer as result. The function in which the web method was called was required by an interface to return a boolean value for success or failure. In my case it came down to something like this:

...

ReturnValue = SAPWebserviceProxy.Z_XXXXXX(...);
ErrorFlag = IsError(ReturnValue);
if(ErrorFlag)
{
LogSAPError(ReturnValue);
}
else
{
LogSuccess(.....);
}

return ErrorFlag;

This is by no means great code, but at least everybody should be able to see what's going on. It's alwys a little awkward, no matter what you do.

A while ago he asked me what he should have printed on my business cards. I said 'Wizard'. I read books which nobody else understand. Then I do something which nobody understands. After that the computer does something which nobody understands. When asked, I say things about the results which nobody understand. But everybody expects miracles from me on a regular basis. Looks to me like the classical definition of a wizard.

[geoffs]

I would have no problems with how you coded that up.

[Rage]

For embedded people, this is the misra conform code :

if (0!=intVar)
m_boolVar=true;
else
m_boolVar=false;

Your first example breaks at least three rules.

[Nemanja Trifunovic]

geoffs wrote:

but it is a coding horror for my programming style and I am curious as to what the others here think about it.

As you said - it is a horror for your programming style. Different people use different styles, and as long as the code is correct and reasonably readable there is no point in discussing it.

Programming Blog utf8-cpp

[Paul Conrad]

It's questionable. Depends on how much readability is necessary.

"The clue train passed his station without stopping." - John Simmons / outlaw programmer "Real programmers just throw a bunch of 1s and 0s at the computer to see what sticks" - Pete O'Hanlon "Not only do you continue to babble nonsense, you can't even correctly remember the nonsense you babbled just minutes ago." - Rob Graham

[realJSOP]

I would have scoped it:

m_boolVar = (intVar != 0);

"Why don't you tie a kerosene-soaked rag around your ankles so the ants won't climb up and eat your candy ass..." - Dale Earnhardt, 1997
-----
"...the staggering layers of obscenity in your statement make it a work of art on so many levels." - Jason Jystad, 10/26/2001

[dighn]

I think it's too trivial to fuss over.

[geoffs]

Which example are you pointing at? The original line of code that I didn't like, or my preferred coding style? Also, which MISRA guidelines are you utilizing... MISCRA-C 1998, MISRA-C 2004, MISRA-C++?

[Swinefeaster]

i would go with:

m_boolVar = intVar ? true : false;

[Lost User]

Thanks :) In any case, a simple method like

public static bool IsError(int ResultValue)
{
// 0 = success
// 1 = some error
// 2 = serious error
// 3 = ....
return ResultValue != 0;
}

will do the trick nicely. Such methods may appear a bit ridiculous as well, but actually the compiler should inline them when optimizing the code. And in cases when the return values represent not only success or failure, but also warnings, correctable errors, critical errors, fatal errors and who knows what else, a method to classify the result is much better than dealing with this redundantly in the code every time it's needed.

A while ago he asked me what he should have printed on my business cards. I said 'Wizard'. I read books which nobody else understand. Then I do something which nobody understands. After that the computer does something which nobody understands. When asked, I say things about the results which nobody understand. But everybody expects miracles from me on a regular basis. Looks to me like the classical definition of a wizard.

[Kevin McFarlane]

I use the style of your second example though usually with brackets around the expression. m_boolVar = (intVar != 0); However, recently I've been moving towards dispensing with the brackets too, partly prompted by my refactoring tool.

Kevin

[Kevin McFarlane]

geoffs wrote:

For me, verbose code is actually painful to read

It's a question of striking the right balance. I often find that too many parentheses makes code harder to read in cases where removing those parentheses would cause no human ambiguity. e.g., I prefer if (a == b || c == d) to if ((a == b) || (c == d)) Similarly I prefer return a == b; to return (a == b);

Kevin

[Kevin McFarlane]

That would be invalid in C# or Java. Even in C/C++ I would always query against a Boolean expression for conceptual clarity.

Kevin

[Andrew Torrance]

I prefer your coworkers style . In terms of efficiency i doubt if there is any significant difference unless this is getting called repeatedly , and it is much much easier to read and understand than m_boolVar = intVar != 0; as this requires you to think about precedence. Just becasue you understand it does not mean that the maintenance programmer will . But at the end of the day its a style question , and nothing divides programmers more than that .

[geoffs]

Now that I've gotten this off of my chest, I can admit to myself that it really was more of a style issue rather than a coding horror as modern compilers would probably generate similar, if not same, code for any of the alternatives. I probably should have posted it to Soap Box or Lounge, but it is what it is at this point. Thanks for all of your inputs. Some people agreed, some did not, and yet others were bored by the whole thing (in which case why did they even bother to reply?). Meanwhile, my style is best... :doh: ;)

[leonej_dt]

geoffs wrote:

m_boolVar = !!intVar;

What is so perverse with that?

To those who understand, I extend my hand. To the doubtful I demand: Take me as I am. Not under your command, I know where I stand. I won't change to fit yout plan. Take me as I am.

[Rage]

The first line. Your favorite style is worse, since you are using implicit casting and poor maintainable conventions. I refer to MISRA-C 2004.

[geoffs]

:laugh: Ah, a man after my own heart! It is not perverse to me. In fact, I am quite at home with that syntax and like it. It is perverse because so many of my fellow programmers hate it and my use of it makes them froth at the mouth or sit there with a dumbfounded look because they don't understand what it is doing.