Click to See Complete Forum and Search --> : Why you shouldn't give advice


Gabriel Fleseriu
October 4th, 2003, 06:33 PM
I could bang my head against the keyboard. Repeatedly. Just spent two hours to debug a piece of code that seemed trivial. Almost begun to suspect "that famous bug of the compiler, that you always seem to discover when your code behaves strange".

Do you see it? It's in the ctor. Unbelivable that I wrote an article about vectors...

class coordinates
{
public:
coordinates():data(3){};
coordinates(double x_, double y_, double z_)
{
data.reserve(3);
data.[0] = x_;
data.[1] = y_;
data.[2] = z_;
}
double& x(){return data[0];}
double& y(){return data[1];}
double& z(){return data[2];}
double x()const{return data[0];}
double y()const{return data[1];}
double z()const{return data[2];}

double& operator[](int i){return data[i];}
double operator[](int i) const{return data[i];}
operator double*(){return &data[0];}
bool operator ==(const coordinates& rhs)const{return (x()==rhs.x()) && (y() == rhs.y()) && (z() == rhs.z());}
bool operator !=(const coordinates& rhs)const{return !(*this==rhs);}
coordinates & operator +=(const coordinates & rhs)
{
x()+=rhs.x();
y()+=rhs.y();
z()+=rhs.z();
return *this;
}
coordinates & operator -=(const coordinates & rhs)
{
x()-=rhs.x();
y()-=rhs.y();
z()-=rhs.z();
return *this;
}
coordinates operator-(){return coordinates(-x(), -y(), -z());}
double length()const{return sqrt(data[0]*data[0] + data[1]*data[1] + data[2]*data[2]);}
void normalize()
{
double l = length();
if(!l){
math_error err;
throw err;
}
else{
data[0]/=l;
data[1]/=l;
data[2]/=l;
}
}
coordinates normalized()const{coordinates temp(*this); temp.normalize(); return temp;}
private:
std::vector<double> data;
};

Yves M
October 4th, 2003, 09:00 PM
It should be resize(3), but I don't quite see how this would break the code, unless the STL you are using throws exceptions (or generates some other error) when using the [] operator. By the way, couldn't you juste rewrite that constructor as:

coordinates(double x_, double y_, double z_) : data(3)
{
data.[0] = x_;
data.[1] = y_;
data.[2] = z_;
}


P.S. Come on, every programmer makes mistakes, don't worry ;) At my company they have an ongoing joke about some code I once wrote that looks like this:

void f(const int i, const int c)
{
if (i == 0) {
// some statements
if (c > 1) {
// some statements
if (i == 0) {
// correct code
} else {
// very bad code (which is luckily never called ;) )
}
}
}
}


P.P.S. Actually, precisely this problem with using reserve instead of resize has been the subject of a GotW item (http://www.gotw.ca/gotw/074.htm) by Herb Sutter, so you can feel even less bad ;)

Sam Hobbs
October 5th, 2003, 12:18 AM
I don't understand why the subject is Why you shouldn't give advice.

Andreas Masur
October 5th, 2003, 05:35 AM
Well...there are all-time favourites like...

unsigned long UnsignedLong = 0;

if(UnsignedLong < 0)
...

It happens from time to time... :cool:

Gabriel Fleseriu
October 5th, 2003, 10:29 AM
Originally posted by Yves M
It should be resize(3), but I don't quite see how this would break the code...

Well, yes, any other solution is good: either resize(), or the vector's ctor. And yes, it breaks the code in a very subtle way: using reserve() followed by operator [] puts the vector in an inconsistent state. The class will behave as if its copy ctor (the default one) was broke :)

Sam: what I meant is "Why you shouldn't give advice unless you also follow it". I'm explicitely pointing out this mistake in my article, so my first thought as I found the bug was "look who was giving advice..." ;)

Yves M
October 5th, 2003, 05:57 PM
Originally posted by Gabriel Fleseriu
Well, yes, any other solution is good: either resize(), or the vector's ctor. And yes, it breaks the code in a very subtle way: using reserve() followed by operator [] puts the vector in an inconsistent state. The class will behave as if its copy ctor (the default one) was broke :)

True, I hadn't though about that. Ah, now I understand why you said you found that infamous compiler bug which generates wrong copy constructors :p

Gabriel Fleseriu
October 5th, 2003, 06:44 PM
Originally posted by Yves M
...you found that infamous compiler bug ...

Yup. Remember, people: the probability ratio that the the compiler is wrong/you are wrong is 0.001% -- if you are lucky ;)

hometown
October 5th, 2003, 06:51 PM
Hope England will be the next, I am sad...

Gabriel Fleseriu
October 5th, 2003, 07:03 PM
Originally posted by hometown
Hope England will be the next, I am sad...

What are you talking about?

Sam Hobbs
October 5th, 2003, 08:06 PM
Originally posted by hometown
Hope England will be the next, I am sad... Next what?

galathaea
October 7th, 2003, 03:12 PM
You know, when I first started reading these forums, I was a complete c++ data structures noob. It was only because I lurked around on the c++ forums and always read people like Gabriel, Yves, Philip, and the Pauls suggesting using STL that I ever even found out about the standard library containers. I had written code about a year and half ago completely with c-style linked lists and repeated code for each class I wrote that required container ability myself, with all the debugging, etc. that ensues. I looked over it about 2 months ago and was horrified! Most of what I had written could be made to perform much faster (it was numerical work) with about a tenth of the code.

So... while you beat yourself up over the error, take a moment to pat yourself on the back as well. The CodeGurus here have taught me way too much for me to ever repay them...

souldog
October 8th, 2003, 03:18 AM
I am going to have to ditto Galathea on this one. It was Paul
McKenzie's posts that lead me to STL containers and Gabriel's
article was the first thing I read on the topic. I read it, wrote a
little program to get a hold of the syntax and was sold.