unexpected template behavior

I have a template class that works fine when I implement it with <int>,
but when I use <floator <doubleit doesn't work. The class has a
dynamic array of type T that gets instantiated in my constructor. When
type T is int, the array works like I expect. But when I use double or
float, the array points to garbage and any updates to array elements
make no difference.

I'm using Visual Studio 2005 on an XP pro machine. The following is
the code that is giving me problems. If anyone can help explain what
I'm doing wrong and why this doesn't work, I'd really appreciate it.
Thanks in advance.


template <class T>
class Tuple
{
public:
Tuple<T>(void);
Tuple<T>(T x, T y);
~Tuple<T>(void);

T getX();
T getY();

void setX(T x);
void setY(T y);
private:
T *_values; //arrray of values
};



template <class T>
Tuple<T>::Tuple(void)
{
_values = new T[2];
_values[0] = 0;
_values[1] = 0;
}

template <class T>
Tuple<T>::Tuple(T x, T y)
{
_values = new T[2];
_values[0] = x;
_values[1] = y;
}

template <class T>
Tuple<T>::~Tuple(void)
{
}

template <class T>
T Tuple<T>::getX() { return _values[0]; }

template <class T>
T Tuple<T>::getY() { return _values[1]; }

template <class T>
void Tuple<T>::setX(T x) { _values[0] = x; }

template <class T>
void Tuple<T>::setY(T y) { _values[1] = y; }

hogtiedtoawaterbuffalo@gmail.com wrote:Hmm.. I used VC++ 2005 Express edition on an XP machine and this code
seemed to work fine for me.

hogtiedtoawaterbuffalo@gmail.com wrote:
I am willing to bet that your test code has undefined behavior. Your
template class has serious issues:

a) The compiler provided copy constructor and assignment operator will copy
pointers instead of the tuples pointed to. Thus your class has reference
semantics instead of value semantics.

b) The memory new[]ed in the constructor is never delete[]d.

Both pitfalls will very likely cause bugs that manifest themselves in UB.
Thus, all bets are off.


Best

Kai-Uwe Bux

Kai-Uwe Bux wrote:
He is using a dynamic array... so if he used the copy constructor, he
would copy the reference to that array... so basically he would re-use
the existing array. This may be an unintentional bug (or exactly what
he wants, I don't know his semantics), but it would not cause a
difference between an array of ints, floats and doubles.

However, use of the copy constructor may indeed provide unextpected
behavior. If the testcode for ints and floats/doubles were somehow
different and the copy constructor was used in the testcode, this could
be a problem area
You would leak the memory if the Tuple is deleted, which may
eventually result in UB, but still doesn't explain a difference between
floats/doubles and ints.

bjeremy wrote:
Since there is nothing in the code posted that would explain a difference
between int and float/double, I conjectured that the test code invokes UB.
Thus, I pointed out the "features" in the posted snippet that may cause
test code to be undefined.


Best

Kai-Uwe Bux

hogtiedtoawaterbuffalo@gmail.com wrote:

If you are not learn design of classes, but learn C++ class's
implementation, then
replace:
to

T getX()const;
T getY()const;

and
to
void setX(const T& x);
void setY(const T& y);

and
to

/*Tuple<T>::*/ enum { item_x=0, item_y, total_items };

template <class T>
Tuple<T>::Tuple():_values(0)
{
//new must not return NULL
_values = new T[total_items];
_values[item_x] = 0;
_values[item_y] = 0;
}

and
to

template <class T>
Tuple<T>::Tuple(const T& x, const T& y):_values(0)
{
//new must not return NULL
_values = new T[total_items];
_values[item_x] = x;
_values[item_y] = y;
}

and
to

template <class T>
Tuple<T>::~Tuple(){ delete _values; _values=0; }

and
to

template <class T>
T Tuple<T>::get?()const { return _values[item_?]; }

and
to

template <class T>
void Tuple<T>::set?(const T& ?) { _values[item_?] = ?; }

And create at least

template <class T>
class Tuple
{
....
private:
Tuple(const Tuple& ):_values(0){abort();}
void Tuple<T>::operator = (const Tuple& ){abort();}
};

And if you are using exceptions, define throw(type) for all class's
members.
Do printing of data for set/get members.

hogtiedtoawaterbuffalo@gmail.com wrote:

If you are not learn design of classes, but learn C++ class's
implementation, then
replace:
to

T getX()const;
T getY()const;

and
to
void setX(const T& x);
void setY(const T& y);

and
to

/*Tuple<T>::*/ enum { item_x=0, item_y, total_items };

template <class T>
Tuple<T>::Tuple():_values(0)
{
//new must not return NULL
_values = new T[total_items];
_values[item_x] = 0;
_values[item_y] = 0;
}

and
to

template <class T>
Tuple<T>::Tuple(const T& x, const T& y):_values(0)
{
//new must not return NULL
_values = new T[total_items];
_values[item_x] = x;
_values[item_y] = y;
}

and
to

template <class T>
Tuple<T>::~Tuple(){ delete _values; _values=0; }

and
to

template <class T>
T Tuple<T>::get?()const { return _values[item_?]; }

and
to

template <class T>
void Tuple<T>::set?(const T& ?) { _values[item_?] = ?; }

And create at least

template <class T>
class Tuple
{
....
private:
Tuple(const Tuple& ):_values(0){abort();}
void Tuple<T>::operator = (const Tuple& ){abort();}
};

And if you are using exceptions, define throw(type) for all class's
members.
Do printing of data for set/get members.

On 2007-01-04 03:31, hogtiedtoawaterbuffalo@gmail.com wrote:Unless you really want the functionality that bjeremy pointed out (when
copying Tuples) I can't see any good reason to use a dynamic array to
store the elements. Alternatives are to use an array which is a member
(non-dynamic) or even better pure members:

T _values[2];

or

T first;
T second;

Either way you don't have to worry about memory leaks.

As for the changes of behavior; use the debugger. VS2005 has excellent
debugging facilities.

--
Erik Wikström

Thanks for the help everyone. Unfortunately, nothing I change seems to
be helping, so perhaps my issue is related to the code I'm using to
debug the class. I tried running the following code on my original
code, then adding the copy constructor and "delete []_values;" to the
deconstructor, and then implenting all the changes that Grizlyk
suggested, and I got the same results each time. I've also tried this
on multiple machines running XP pro and also tried it with Visual
Studio 2005 and Visual Studio C++ Express Edition - no luck. Clearly
I'm doing something wrong. Any thoughts?

#include "Tuple.h"
#include <stdio.h>

int main()
{
Tuple<inttup1(1, 2);
printf("tup1: %i, %i\n", tup1.getX(), tup1.getY()); //prints as
expected "tup1: 1, 2"

Tuple<floattup2(2.2f, 2.5f);
printf("tup2: %d, %d\n", tup2.getX(), tup2.getY()); //prints garbage

getchar(); //stop the console window from closing
return 0;
}


Just to clarify, all of my class code is in the Tuple.h file, it is not
split between .h and .cpp.

Again, thanks in advance to any responses.

hogtiedtoawaterbuffalo@gmail.com wrote:
Because %d is not the formatting character for floating point numbers.

#include <iostream>
#include <cstdio>
int main(void)
{
double x = 5.5;
printf("%d", x);
std::cin.get();
}

I get 0 but it's totally unpredictable what will happen on any given
computer.

Noah Roberts wrote:Well... Kai was right, it looks like its your test code... When I was
testing your code I was using std::cout to print out the array
values... you may want to use std::cout instead of printf, and this is
atually a good example of why...

Changing from printf to std::cout solved everything. Sheesh!

Thanks for the help everyone.