What's Wrong with this Code?

Article
06/13/2006

A while back, Erik Schwiebert wrote about some of the travails of moving our build system from one using CodeWarrior to one using XCode and GCC. Erik mentioned the huge number of errors and warnings we've had to resolve, but, in reading some of the reactions around the web, I'm afraid that some people, particularly people who know nothing about compilers and developing software, seem to have completely mistaken Erik's remarks.

Someday, I'll understand why people are motivated to render opinions on matters of which they are entirely ignorant, but, for now, I'll have to satisfy myself by offering some remedial education. I'll start by asking, what's wrong with the following code:

 
class FooBar
{
public:
    FooBar(int val): _val(val) {};
    ~FooBar() {};

    void Foo(int val)
        {
        _val += Bar(val);
        };

    int Val() { return _val; };

private:
    int Bar(int val)
        {
        _val = val;
        return _val;
        };

    int _val;
};

int main()
{
    FooBar fb(42);
    fb.Foo(1);

    int val = fb.Val();

    return 0;
}

Some of my readers will spot the problem immediately. To those who do, I'll remind you that this is a contrived example. A real-world example will have a class that has 10 or more methods, and none of those methods will be as trivial as Foo or Bar.

Update: Fixed code problems due to .Text's handling of HTML entities.

Rick

Currently playing in iTunes: One Way Out by The Allman Brothers

Comments

Anonymous
June 13, 2006
The result of the += is undefined, as there's no guarantee of the order of subexpressions, ond one of those affects the other?
Anonymous
June 13, 2006
Am I missing something here? Val() will return 2 because Bar() just assigns val to _val.

Perhaps you intended to call Bar() from the ctor() or something along those lines?
Anonymous
June 13, 2006
The comment has been removed
Anonymous
June 13, 2006
I'm not going to give it away entirely, but I think the problem is +=, specifically Stroustrup 6.2.2 - Evaluation Order. Let me guess - CodeWarrior gives 43, and XCode gives 2, or vice-versa. Ugly.
Anonymous
June 13, 2006
The comment has been removed
Anonymous
June 13, 2006
Jason,

There is only one ctor, so you can't have _val uninitialized.

Joshua,

You had a 50% chance of getting it right. Bummer :-).
Anonymous
June 13, 2006
PingBack from http://www.schwieb.com/blog/archives/22
Anonymous
June 13, 2006
The comment has been removed
Anonymous
June 13, 2006
Darn it, Schwieb, you're stealing the thunder of my next post! :-).

Oh, and minus points for using printf. Next time, include iostream, and use:

std::out << fb.Val() << std::endl;
Anonymous
June 13, 2006
The comment has been removed
Anonymous
June 14, 2006
The comment has been removed
Anonymous
June 14, 2006
Frederik, yes, the semicolons at the end of the method definitions are optional. I tend to add them simply out of habit, but that's not strictly a coding error.

Jason, there is no cannonical coding style for braces. Use whichever style you find most comfortable and/or whichever style is dictated by the group you're in. I used the style we tend to use in our code, but that doesn't mean the style I used is cannonically correct in any way.
Anonymous
June 14, 2006
Bjarne Stroustrup is a brilliant man, which means he's very good at defining programming languages. ...
Anonymous
June 14, 2006
@Joshua: What Stroustrup book are you referring to? My copy of "The Design and Evolution of C++" (4th printing) has chapter 6.2.2. named "How does the Committee Operate?". "The C programming language" (by K&R, second edition) has only a chapter 6.2 and it's about structures and functions.

I'm not sure what you're referring to?

@Rick: It's an insiduous problem, but I would have thought that by now ANSI and ISO would have discovered this and standardised on one interpretation? Of course, two compilers can still have the bug for legacy reasons, but does the standard really leave this undefined?
Anonymous
June 14, 2006
Uli, Joshua is referring to Stroustrup's, "The C++ Programming Language."

And, yes, the standard really does leave it undefined. This has to do with the age-old efficiency trade-off. Allowing compiler writers to choose the order of subexpression evaluation as they see fit enables them to take advantage of certain processor dependant issues. The most common case involves instruction scheduling. You'll often see compilers changing evaluation order based on whether the operands are allocated in registers or allocated on the system stack.
Anonymous
June 14, 2006
PingBack from https://blogs.msdn.com:443/rick_schaut/archive/2006/06/14/630976.aspx
Anonymous
June 14, 2006
Here's a link I found that explains the difference between "implementation-defined", "unspecified", and "undefined" behavior.

http://c-faq.com/ansi/undef.html

Might be useful for folks who wonder why compilers can even be allowed to be so vague.
Anonymous
June 14, 2006
The comment has been removed
Anonymous
June 13, 2009
PingBack from http://barstoolsite.info/story.php?id=73
Anonymous
June 16, 2009
PingBack from http://fixmycrediteasily.info/story.php?id=3840
Anonymous
June 16, 2009
PingBack from http://lowcostcarinsurances.info/story.php?id=7475

Share via

What's Wrong with this Code?

Comments

Additional resources