Re: BrValueObject in CVS

From: Christian Holm Christensen (cholm@hehi03.nbi.dk)
Date: Mon Nov 20 2000 - 16:20:20 EST

  • Next message: Konstantin Olchanski: "Re: BrValueObject in CVS"

    Hi Bjorn et al, 
    
    On Fri, 17 Nov 2000 03:24:21 -0500 (EST)
    Bjorn H Samset <bjornhs@rcf.rhic.bnl.gov> wrote
    concerning ": BrValueObject in CVS":
    > 
    > Ping. As the only response I got was positive, I have added 
    > /base/inc/BrValueObject.h
    > /base/src/BrValueObject.cxx
    > in CVS.
    
    Damn. I guess that's the price you've gotta pay for taking a short
    vacation. 
    
    I DO NOT LIKE THE IDEA OF BrValueObject!!!!! 
    
    1) It's too big:
       
       sizeof(BrDataObject)              144 Bytes 
       sizeof(Double_t)                    8  -"-
       sizeof(Float_t)                     4  -"-
       sizeof(Int_t)                       4  -"-
       sizeof(Byte_t)                      1  -"- 
       -------------------------------------------
       sizeof(BrValueObject)             161 Bytes
    
       The size of TObject is only 12 Bytes, so you should rather inherit
       from that. This actually raises the question why we at all bother
       to have BrDataObject; I know Flemming/Kris/??? once wanted to do
       something like what Phenix is doing, try to hide ROOT completly
       from the user so that BRAHMS could switch to any analysis
       toolkit/framework should the need arise. However, I believe we have
       commited ourselves - for better or for worse - to ROOT, which I
       personally think is wise. In fact, I think we've all reached a
       level of proficiency in ROOT/C++ that we really want to have ROOT
       closer to the surface. 
    
    2) Specialised classes is prefered. If you really need a summary
       object of some sort, then you should make such a class. 
    
    3) BrValueObject is too versatile. I know this sounds a bit odd, but
       think about it for a while. Most of the time, you only need to
       store a Double_t,Int_t, OR Byte_t, not all of them at the same
       time. Hence, what you could have done - I'm NOT encouraging you to
       do so - was to create the classes BrDouble, BrInt, BrByte. Also,
       you got both a Float_t AND a Double_t, which is completly
       redundant. Stick to one floating point type (preferably double
       precision)! 
    
    4) There's a slightly odd but very simple way to store a simple number
       in ROOT. Take a look at what Rene had to say on this:
    
          http://root.cern.ch/root/roottalk/roottalk00/2311.html
    
       His suggestion is perfect for debugging stuff and so on, but
       probably not for production - but in production mode, you'd better
       make a specialised class, or if that's seems to much, the data
       isn't worth storing in the first place. 
    
    I suggest you remove BrValueObject from BRAT ASAP. Here's how to if
    you don't know it already:
    
      prompt% klog -principal bjornhs -cell rhic 
      prompt% cd $BRATHOME  
      prompt% rm base/inc/BrValueObject.h base/src/BrValueObject.cxx
      prompt% cvs remove base/inc/BrValueObject.h base/src/BrValueObject.cxx
      prompt% cvs ci -m "Removed class BrValueObject" \
        base/inc/BrValueObject.h base/src/BrValueObject.cxx
    
    > Flemming: I have also added the if (DebugLevel()>0)-statements to the
    > error-messages in BrTPMTrackVertexModule.cxx
    
    Cheers! Greatly appriciated! 
    
    Yours, 
    
    Christian  -----------------------------------------------------------
    Holm Christensen                             Phone:  (+45) 35 35 96 91 
      Sankt Hansgade 23, 1. th.                  Office: (+45) 353  25 305 
      DK-2200 Copenhagen N                       Web:    www.nbi.dk/~cholm    
      Denmark                                    Email:       cholm@nbi.dk
    



    This archive was generated by hypermail 2b29 : Mon Nov 20 2000 - 16:21:41 EST