Re: [Brahms-dev-l] c++ question

From: Christian Holm Christensen <cholm@nbi.dk>
Date: Fri Aug 27 2004 - 19:00:07 EDT
Hi Peter and Erik, 

On Fri, 2004-08-27 at 18:07 +0200, Peter H.L. Christiansen wrote:
> Hi Erik,
> 
> You have to make it a pointer to a pointer **, Root has a TMatrix class 
> that might be useful....

`Pointer to pointer' - Yikes, talk about bad C++.   Peter's suggestion
of using a proper class like `TMatrix' is a very good idea. 

> void Main ()
> {
>   Float_t SiPhi[6][42];

Actually, this is not a pointer-to-pointer.  It's an array of arrays,
and it'll seize to exist once you leave the scope.  A pointer-to-pointer
would be more like 

   float** siPhi = new float*[6];
   for (size_t i = 0; i < 6; i++) siPhi[i] = new float[42];

which you'd have to delete by 

   delete [] siPhi;

>   LoadPhiMap(SiPhi);
>   for (Int_t i = 0; i<6; i++) {
>     for (Int_t j = 0; j<42; j++) {cout<<SiPhi[i][j]<<endl;}
>   }
> }
> 
> void LoadPhiMap(Float_t **SiPhi)
> {
>   
> }

OK, I can't believe you actually did a copy'n'paste of your source file
for this.   First off, the name of the entry point is `main' not `Main',
second off, it _must_ evaluate to `int', and third, you haven't declared
`LoadPhiMap' before you actually use it, so my guess is, that you're
actually making a ROOT script.  There are subtle differences, so it's
important to know precisely what you do if we are to give you any
meaningful advice. 

Also, you have a semantic error in the second loop.  You're incrementing
the index `i' when you really wanted to increment `j', and you're
testing whether `i' is less than 42, while it's really `j' you want to
check - a very common mistake when one does blind copy'n'paste.  Peter,
having a keen eye, did however correct that above. 

So, to make your code acceptable to a compiler, you'd write 

  #include <iostream>
  #include <TMatrix.h>

  bool LoadPhiMap(TMatrix& m);

  int main() 
  {
    TMatrix siPhi(6, 42);
    if (!LoadPhiMap(siPhi)) {
      std::cerr << "Error loading the SMA phi map" << std::endl;
      return 1;
    }
    for (size_t i = 0; i < siPhi.GetNrows(); ++i) 
      for (size_t j = 0; j < siPhi.GetNcols(); ++j) 
        std::cout << m(i,j) << std::endl;
    return 0;
  }

  bool LoadPhiMap(TMatrix& m) 
  { 
     if (m.GetNrows() < 6 || m.GetNcols() < 42) return false;
     for (size_t i = 0; i < siPhi.GetNrows(); ++i) 
       for (size_t j = 0; j < siPhi.GetNcols(); ++j)
	 m(i,j) = i * 6 + j;
     return true;
  }

Of course, using a proper STL container, like
`std::map<std::pair<size_t,size_t>, float>' is a possibility too,
especially if the map is sparse. 


> > I get this error 
> > Error: Function LoadPhiMap(SiPhi) is not defined in current scope
> > Does anyone know how to handle 2d arrays in this situation?

My general advice:  Stay clear of pointers as much as you can, and if
you find yourself using multi-subscripted arrays or pointer-to-pointer,
you're doing something wrong, and you need to take a closer look at your
code and the API you're using. 

(Note, that using a pointer to a TMatrix object in `LoadPhiMap' would be
wrong - there's no way you'll ever want to pass the null matrix to that
function.) 

Yours,

-- 
 ___  |  Christian Holm Christensen 
  |_| |  -------------------------------------------------------------
    | |  Address: Sankt Hansgade 23, 1. th.  Phone:  (+45) 35 35 96 91
     _|           DK-2200 Copenhagen N       Cell:   (+45) 24 61 85 91
    _|            Denmark                    Office: (+45) 353  25 404
 ____|   Email:   cholm@nbi.dk               Web:    www.nbi.dk/~cholm
 | |


_______________________________________________
Brahms-dev-l mailing list
Brahms-dev-l@lists.bnl.gov
http://lists.bnl.gov/mailman/listinfo/brahms-dev-l
Received on Fri Aug 27 19:00:30 2004

This archive was generated by hypermail 2.1.8 : Fri Aug 27 2004 - 19:00:47 EDT