[SOLVED] accessing private vars in template class

[monkeyking]

Hi I'm doing a template container class abstraction of a math vector.
That should have some basic math stuff.

Now the problem is, that I can access private vars in other instatiations, but not in all cases.

The copyconstructor works but not the comparison operator.

I can of cause just make a const accessor, but I'd like to avoid the function call overhead.

But I don't really understand the problem,
I don't understand why compileren allows access to the private variables sometimes, and sometimes not.
Some consistency would be nice
thanks in advance

#include <iostream>

using namespace std;

template<typename T>
class Array {
 public:
  Array() {data_=NULL;x_=0;numOnes_=0;}
  Array(int length):x_(length),data_(new T[length]), numOnes_(0){}
  Array(const Array<T>& var);
  ~Array(){delete [] data_;}
  int length() const                  { return x_;     }
  T& operator() (uint r); 
  T operator() (uint r) const; 

  void print(int x=0, char sep=' ');
  void fillUp(T var) {for(int i=0;i<x_;i++) data_[i]=var;}
  Array<char> operator< (const float &f);
  int numTrues() const {return numOnes_;}

  Array<T> extract(const Array<char>&other);

private:
  int x_;
  T*  data_;
  int numOnes_;
};


template<typename T>
Array<T>::Array(const Array<T>& var){
  puts("array const constructor");
  x_ = var.x_;//this works
  data_ = new T[x_];
  for(int i=0;i<var.x_;i++)
    data_[i]=var.data_[i];
  
}

template<typename T>
void Array<T>::print(int x,char sep){
  printf("printing array with dim=%d\n",x_);
    for(int i=0;i<x_;i++) 
      std::cout << data_[i] << sep;
  std::cout <<endl;
}

template<>
void Array<char>::print(int x,char sep){
  printf("printing array with dim=%d\n",x_);
    for(int i=0;i<x_;i++) 
      std::cout <<(int) data_[i] << sep;
  std::cout <<endl;
}


template <typename T>
T& Array<T>::operator() (uint r){
  return data_[r];
} 


template <typename T>
T Array<T>::operator() (uint r) const{
  return data_[r];
} 

//these are comparison operators, these will return a Array<char> with elems {0,1}
template<class T>
Array<char> Array<T>::operator< (const float  &f){
  Array<char> tmp(x_);

  for(int i=0;i<x_;i++){
    if (data_[i]<f){
      tmp.data_[i]=1;
      tmp.numOnes_++;//this doesn't work
    }
    else
      tmp.data_[i]=0;
  }
  return tmp;

}

int main(){
  Array<int> vars(5);
  vars(0)=5;vars(1)=1;vars(2)=4;vars(3)=5;vars(4)=0;
  vars.print();
  Array<char> posList = vars<3;
  posList.print();
} 

[dribeas]

//these are comparison operators, these will return a Array<char> with elems {0,1}
template<class T>
Array<char> Array<T>:: operator< (const float  &f){
  Array<char> tmp(x_);

  for(int i=0;i<x_;i++){
    if (data_[i]<f){
      tmp.data_[i]=1;
      tmp.numOnes_++;//this doesn't work
    }
    else
      tmp.data_[i]=0;
  }
  return tmp;
} 

Template instantiations (note: not templates, but the instantiations) are classes and behave as such. This means that Array<int> is a class, and Array<char> is a class, but they are different classes, even if you wrote the code only once.

The problem with your template is that class Array<int> is trying to access private members of Array<char> (which is a different class).

There are a couple of solutions, first would be providing accessors, but that would open the internal data to just about anyone. The function call overhead would not really be the problem, as you can make you accessors inlined. The problem would be encapsulation.

template <typename T>
class Array
{
public:
   // ...
   T* data() { return data_; }
   int& ones() { return numOnes_; }
};

void anotherPieceOfCode()
{
   Array<int> array;
   delete array.data(); // !! problem external code can corrupt your internal data!!!
}

Another solution is using friendship:

template <typename T>
class Array
{
// ...
   template <typename U>
   friend Array<char> Array<U>::operator<( float const & );
};

This has better access control, not all code can access your internals, only operator< from any other Array<> template instantiation. The good point is that you are limiting the access to only a small (controlled) subset of classes, the bad parts are that you are still opening your code more than you should:

- Any Array<T>:: operator< can access any other Array<U> internals, that is Array<int>:: operator< can access Array<char> internals, but also Array<double> internals, or Array<...>

- Templates are tricky here. Any external programmer can break your encapsulation (not that common people might want to, but it is possible) through the use of template specialization.

Anyway, the difference is that in this case, a malicious coder must go out of the way to break your system, while in the first case, with accessors any code can just out of error corrupt your data.

As a side note, why are you overriding operator() instead of operator[] that seems more appropriate? Why are operator() not implemented in the class definition? (if you did, the compiler would be allowed to inline it, and with your implementation that would faster [skip method call] and probably yield smaller binary). Why do you have a numOnes attribute/numTrues() member method? do you plan to use it in all classes or just in your Array<char>?

[monkeyking]

... This means that Array<int> is a class, and Array<char> is a class, but they are different classes, even if you wrote the code only once.

Of cause of cause, that makes perfect sense. This template way of programming is still quite new for me.

template <typename T>
class Array
{
// ...
   template <typename U>
   friend Array<char> Array<U>::operator<( float const & );
};

I'm having problems compiling this syntax, I'm getting this nasty compile error, can you elaborate on syntax.

make
g++ testMatrix.cpp alloc.o -Wall -ansi -pedantic -ggdb
In file included from testMatrix.cpp:7:
array.cpp: In instantiation of ‘Array<char>’:
array.cpp:55:   instantiated from here
array.cpp:19: error: no ‘Array<char> Array<T>::operator<(const float&)’ member
function declared in class ‘Array<T>’
array.cpp:136: error: expected 2 levels of template parms for ‘Array<char>
Array<T>::operator<(const float&)’, got 1
array.cpp: In member function ‘Array<char> Array<T>::operator<(const float&)’:
array.cpp:137: error: ‘x_’ was not declared in this scope
array.cpp:140: error: ‘data_’ was not declared in this scope
array.cpp:19: confused by earlier errors, bailing out
Preprocessed source stored into /tmp/ccyMgsiS.out file, please attach this to your
bugreport.
Traceback (most recent call last):
  File "/usr/share/apport/gcc_ice_hook", line 34, in <module>
    pr.write(open(apport.fileutils.make_report_path(pr), 'w'))
IOError: [Errno 13] Permission denied:
'/var/crash/_usr_lib_gcc_x86_64-linux-gnu_4.2.3_cc1plus.1000.crash'
make: *** [allofit] Error 1 

As a side note, why are you overriding operator() instead of operator[] that seems more appropriate? Why are operator() not implemented in the class definition? (if you did, the compiler would be allowed to inline it, and with your implementation that would faster [skip method call] and probably yield smaller binary). Why do you have a numOnes attribute/numTrues() member method? do you plan to use it in all classes or just in your Array<char>?

I'm overloading'()' since I'm also doing a matrix template,
and the parashift faq said that I should use subsetoperator '(a,b)' instead of '[a][b]'. So I choose '()' just for making the different classes more alike.

I was told, don't remember who though, that I should always avoid method calling when I had the option of accessing the data directly, something about avoid stack swap or something. That's why I don't just use the overloaded '()' method. Should I just do this?

numOnes_/numtrues() is only used in Array<char>, is there some nice way of just having them in Array<char>

I stripped my template class definition, for most stuff when I did my original post.

this is my full 'template class header'

template<typename T>
class Array {
 public:
  Array() {puts("empty constructor");data_=NULL;x_=0;numOnes_=0;}
  Array(int length):x_(length),data_(new T[length]), numOnes_(0){}
  void init(int length) {x_=length;data_=new T[length]; }
  Array(const Array<T>& var);
  ~Array(){delete [] data_;}
  int length() const                  { return x_;     }
  T& operator() (uint r); 
  T operator() (uint r) const; 

  void print(int x=0, char sep=' ');
  void fillUp(T var) {for(int i=0;i<x_;i++) data_[i]=var;}
  void plus(const Array<T>&);
  template <typename U>
  friend Array<char> Array<U>::operator< (const float &f);//this makes nasty compile error
  Array<char> operator> (const float &f);
  Array<char> operator== (const float &f);
  int numTrues() const {return numOnes_;}

  Array<T>& operator= (const Array<T>&);
  Array<T> operator+ (const Array<T>&);
  Array<T> operator/ (const float &other);
  
  Array<T> extract(const Array<char>&other);

private:
  int x_;
  T*  data_;
  int numOnes_;
}; 

thanks for your response

[dwhitney67]

Monkeyking, although this suggestion has nothing to do with the coding issues you are facing, I would suggest that you use more white-space (and vertical space) when you write code.

Your code is very to hard to read.

[monkeyking]

Monkeyking, although this suggestion has nothing to do with the coding issues you are facing, I would suggest that you use more white-space (and vertical space) when you write code.

Your code is very to hard to read.

Ok, I'll indent by 8 the next time.
It's not that I like writing unreadable code,
but when I started to program some years ago,
my teachers wanted a printout.
So I just got used to use 2 space tab and no lines with more than 78 chars wide.

It's just a bad habit.

[monkeyking]

Ok,         I'll     indent    by    8    the     next     time.    :)
It's        not      that   I   like    writing     unreadable code   ,
but     when     I    started    to   program    some   years    ago  ,
my    teachers    wanted   a    printout       .
So      I      just       got        used      to       use      2 space   tab         and   no    lines    with    more    than    78    chars   wide  .

It's    just     a     bad      habit  .

[dwhitney67]

It's not the indent that vexes me; I personally only indent two-spaces too.

It's stuff like:

for(int i=0;i<5;++i) 

versus

for (int i = 0; i < 5; ++i) 

[dribeas]

template<typename T>
class Array {
 public:
   // ...
  template <typename U>
  friend Array<char> Array<U>::operator< (const float &f);//this makes nasty compile error
   // ...
}; 

The problem here is that you are declaring operator< from an external Array<U> class as a friend (which I suggested) but you removed Array<T>:: operator< from the template class. A friendship declaration is not a method declaration, the compiler knows that operator< can have access to the internal data, but it cannot find the operator< anywhere.

class Array {
 public:
   // ...
  Array<char> operator< (const float & f); // declaration of the method
  template <typename U>
  friend Array<char> Array<U>::operator< (const float &f);// friendship declaration for all other Array<U> operator<
   // ...
};

More on the design later on, if I find the time

[dribeas]

numOnes_/numtrues() is only used in Array<char>, is there some nice way of just having them in Array<char>

I don't like the idea of having a member attributes and/or methods in a template if you know before hand that it will be used only in one template instantiation.

Then I don't like the idea of having external code modifying internal data (numOnes_), which is the only solution with your current Array<> interface.

There are a couple of things you can do:
1- Specialize Array<char> and include numOnes_/numTrues()
2- Provide a separate non templated class (Mask?) to use as return value from Array<>:: operator<

In either case you will need to provide most method implementations again, even if it just delegates into the general template Array<>. I don't like from the template specialization that you will have a different behavior for an specific type of Array<>.

Without knowing your particular requirements, so I will probably be way off, I would go for option (2), with an implementation in these lines:

class ArrayMask
{
public:
   ArrayMask( int size ) : data_(0), numOnes_(0) {}
   void set( int pos, bool value = true ) // replaces bool& operator()
   {
      if ( data_(pos) ) --numOnes_;
      data_(pos) = value;
      if ( value ) ++numOnes_;
   }
   bool operator( int pos ) const // most methods can be delegated
   {
      return data_( pos );
   }
   int numTrues() const { return numOnes_; }
   // You'll need to delegate other needed methods as with bool operator(int) const
private:
   Array<bool> data_;
   int numOnes_;
};

template<typename T>
ArrayMask Array<T>::operator< ( float value ) // there is no point in using references for basic types
{
  ArrayMask result( length_ ); // in your code x_
  for ( int i = 0; i < length_; ++i )
  {
    if ( data_[ i ] < value ) result.set( i );
  }
  return result;
}

The change in the interface (replacing non-const operator() with a set method) is to keep control of the 'true' count.

If you want to use Mask in operations with Array<> you can go through a couple of ways: provide operators that work on both types, or provide an accessor (possibly constant) to your inner Array<bool>.

This solution is cleaner, and you don't want to use friendship (it is the strongest coupling relationship) and Array<int> won't need to have attributes/methods you don't need.

On a side note, instead of your print signature, it is recommended having either a friend operator<< or a non-friend operator<< that calls a print/dump method with this signature:

template<typename T>
class Array
{
   // ...
public:
   std::ostream& dump( std::ostream& o, char separator = ' ' ) const
   {
      o << "Array of size " << x_; 
      // ... print all data
      return o;
   }
};

template <typename T>
std::ostream & operator<<( std::ostream & out, Array<T> const & a )
{
   return a.dump( out );
}

This will allow you to use the same method for printing to any kind of stream (stdout/stderr/file stream/string stream...) with just one implementation.

[monkeyking]

The problem here is that you are declaring operator< from an external Array<U> class as a friend (which I suggested) but you removed Array<T>:: operator< from the template class. A friendship declaration is not a method declaration, the compiler knows that operator< can have access to the internal data, but it cannot find the operator< anywhere.

Very very informative, I had to reread this a couple of times before it made sense. But I got it now.

There are a couple of things you can do:
1- Specialize Array<char> and include numOnes_/numTrues()
2- Provide a separate non templated class (Mask?) to use as return value from Array<>:: operator<

I got your idea, and I think I'll go with the second one, but just to clarify.
There is no way to say. Let Array<char> have all the public methods from Array<T> plus some extra internal vars?
Without having to duplicate the general code?

...
This will allow you to use the same method for printing to any kind of stream (stdout/stderr/file stream/string stream...) with just one implementation.

Very clever, this one is indeed a keeper.

Thanks for your reply