Re: [ROOT] TGeoShape GetShapeBit policy?

From: Rene Brun (Rene.Brun@cern.ch)
Date: Tue Feb 24 2004 - 13:16:31 MET


Hi Victor,

I am still very confused by your remark.
IsA privides you in a VERY EFFICIENT WAY a pointer to the TClass
corresponding to any object. This facility is used internally
by many ROOT functions when access to the dictionary is required.
Calling this function should appear rarely in the user's code.
In the case of the TGeo classes, it could be a nice alternative
to having dedicated enums in a base class (this contradicts OO,
since a base class should not know anything about its derived classes).

Looking forward to meet you at the workshop to clarify your remark.

Rene Brun

On 
Mon, 
23 
Feb 2004, Victor 
Perevoztchikov wrote:

> Hi Rene,
> > I do not understand your remarks
> >  -IsA is very fast
> It is virtual method, as any virtual it is not too fast. But for this case,
> I aqgre,  it is not very important.
> 
> >  -It does not contradict OO
> It contradicts to OO very much.
> One of the main idea of OO, inheritance. User can create own class,
> inherited from
> the base, and run the application which is used base class. And application
> does not care which is real implementation.
> But if application uses IsA() and got the user TClass, about which it does
> know nothing,
> it can not work at all. Which is completely contradiction to OO ideology.
> 
> Victor
> 
> 
> Victor M. Perevoztchikov   perev@bnl.gov
> Brookhaven National Laboratory MS 510A PO Box 5000 Upton NY 11973-5000
> tel office : 631-344-7894; fax 631-344-4206;
> 
> ----- Original Message ----- 
> From: "Rene Brun" <Rene.Brun@cern.ch>
> To: "Victor Perevoztchikov" <perev@bnl.gov>
> Cc: "Andrei Gheata" <Andrei.Gheata@cern.ch>; "Valeri Fine" <fine@bnl.gov>;
> <roottalk@pcroot.cern.ch>
> Sent: Monday, February 23, 2004 8:13 PM
> Subject: Re: [ROOT] TGeoShape GetShapeBit policy?
> 
> 
> > Hi Victor,
> >
> > I do not understand your remarks
> >  -IsA is very fast
> >  -It does not contradict OO
> >
> > Rene Brun
> >
> > On Mon, 23 Feb 2004, Victor Perevoztchikov
> > wrote:
> >
> > > Hi Andrei,
> > >
> > > > beginning for a "just in case usage". It is clear that their
> > > > functionality is fully provided by IsA(). To be removed at the next
> > >
> > > I think that GetShapeType() approach
> > > > >>EShapeType TGeoShape::GetShapeType() const
> > > > >>This will eliminate any ambiguity.
> > >
> > > is much better than Isa(). There are two reasons:
> > > 1. IsA rather slow;
> > > 2. It contradicts to Object oriented approach. I am not talking from
> > > "religious" point
> > >     of view, but from just practical. Let say user or even you, invents
> some
> > > specific shape,
> > >    with slightly different functionality. His class is inherited from
> the
> > > standard one.
> > >    In case of GetShapeType() , everything is working fine. But in case
> of
> > > IsA() test
> > >    code will fail.
> > >
> > > So, I think it is better do not use IsA() but TGeoShape::GetShapeType()
> > > const
> > > Victor
> > >
> > >
> > >
> > > Victor M. Perevoztchikov   perev@bnl.gov
> > > Brookhaven National Laboratory MS 510A PO Box 5000 Upton NY 11973-5000
> > > tel office : 631-344-7894; fax 631-344-4206;
> > >
> > > ----- Original Message ----- 
> > > From: "Andrei Gheata" <Andrei.Gheata@cern.ch>
> > > To: "Valeri Fine" <fine@bnl.gov>
> > > Cc: <roottalk@pcroot.cern.ch>
> > > Sent: Monday, February 23, 2004 10:28 AM
> > > Subject: Re: [ROOT] TGeoShape GetShapeBit policy?
> > >
> > >
> > > > Hi Valeri,
> > > >
> > > > You are right. These "shape type" bits were included from the very
> > > > beginning for a "just in case usage". It is clear that their
> > > > functionality is fully provided by IsA(). To be removed at the next
> > > > clean-up.
> > > >   Some other bits (such as kGeoBad or kGeoInvalidShape) are internally
> > > > used to mark shapes that are detected as "malformed" - they are of no
> > > > usage even when trying to convert a TGeo geometry to a different
> system
> > > > since after closing the geometry all shapes in the logical tree are
> valid.
> > > >
> > > > Best regards,
> > > > Andrei
> > > >
> > > > Valeri Fine wrote:
> > > > > Hello Andrei
> > > > >
> > > > >   Thank you very much for your clarification.
> > > > >
> > > > >
> > > > >>The fix to this is to define an abstract method at TGeoShape level,
> > > > >
> > > > > that
> > > > >
> > > > >>any specific shape has to implement, e.g.
> > > > >>
> > > > >>EShapeType TGeoShape::GetShapeType() const
> > > > >>This will eliminate any ambiguity.
> > > > >
> > > > >
> > > > >   What would be the advantage of the method you are going to
> implement
> > > > > against of
> > > > >
> > > > >     if (shape->IsA() == TGeoTrap::Class() )
> > > > >
> > > > > as soon as both TGeoShape and TGeoTrap (for example) classes are
> parts
> > > > > of one and the same package, share library , and RootCint
> dictionary?
> > > > >
> > > > > Do you think to avoid the confusion it would be better to eliminate
> the
> > > > > bits dealing with the concrete implementations (because this
> information
> > > > > is provided via IsA() method anyway)?
> > > > >
> > > > >
> > > > >>>      static const TGeoShape::EShapeType kGeoTorus
> > > > >>>      static const TGeoShape::EShapeType kGeoBox
> > > > >>>      static const TGeoShape::EShapeType kGeoPara
> > > > >>>      static const TGeoShape::EShapeType kGeoSph
> > > > >>>      static const TGeoShape::EShapeType kGeoTube
> > > > >>>      static const TGeoShape::EShapeType kGeoTubeSeg
> > > > >>>      static const TGeoShape::EShapeType kGeoCone
> > > > >>>      static const TGeoShape::EShapeType kGeoConeSeg
> > > > >>>      static const TGeoShape::EShapeType kGeoPcon
> > > > >>>      static const TGeoShape::EShapeType kGeoPgon
> > > > >>>      static const TGeoShape::EShapeType kGeoArb8
> > > > >>>      static const TGeoShape::EShapeType kGeoEltu
> > > > >>>      static const TGeoShape::EShapeType kGeoTrap
> > > > >>>      static const TGeoShape::EShapeType kGeoCtub
> > > > >>>      static const TGeoShape::EShapeType kGeoTrd1
> > > > >>>      static const TGeoShape::EShapeType kGeoTrd2
> > > > >>>      static const TGeoShape::EShapeType kGeoComb
> > > > >
> > > > >
> > > > >
> > > > > What about other bits I would like to understand the meaning of the
> bits
> > > > > with no deep digging of the source code.  For example it is not
> clear
> > > > > from the first glance what is the purpose of the
> > > > >
> > > > >       static const TGeoShape::EShapeType kGeoBad
> > > > > vs
> > > > >       static const TGeoShape::EShapeType kGeoInvalidShape
> > > > >
> > > > > Hope this helps,
> > > > >                     Thank you
> > > > >
> > > > > ----
> > > > > Best regards
> > > > >                    Valeri
> > > > >
> > > > >
> > > > >
> > > > >>-----Original Message-----
> > > > >>From: Andrei Gheata [mailto:Andrei.Gheata@cern.ch]
> > > > >>Sent: Thursday, February 12, 2004 2:59 AM
> > > > >>To: Valeri Fine
> > > > >>Cc: roottalk@pcroot.cern.ch
> > > > >>Subject: Re: [ROOT] TGeoShape GetShapeBit policy?
> > > > >>
> > > > >>Hi Valeri,
> > > > >>
> > > > >>Shape bits should provide the following information:
> > > > >>1. shape type - one might want to cast a shape pointer at run time
> > > > >>2. validity of the shape
> > > > >>3. if shape represent a parametrized object
> > > > >>4. if the shape is closed - e.g. its bounding box is computed
> > > > >>
> > > > >>The first category is not used by the modeller. The shape type
> > > > >>identifiers are set in the constructor. The problem with the
> TGeoTrap
> > > > >
> > > > > is
> > > > >
> > > > >>that it inherits from TGeoArb8 but not setting its own bit. So this
> is
> > > > >
> > > > > a
> > > > >
> > > > >>bug. In fact, it is more an inconsistent feature, since you will
> find
> > > > >>for instance that a TGeoTubeSeg have both kGeoTube and kGeoTubeSeg
> > > > >
> > > > > bits set.
> > > > >
> > > > >>The fix to this is to define an abstract method at TGeoShape level,
> > > > >
> > > > > that
> > > > >
> > > > >>any specific shape has to implement, e.g.
> > > > >>
> > > > >>EShapeType TGeoShape::GetShapeType() const
> > > > >>This will eliminate any ambiguity.
> > > > >>
> > > > >>Soon in CVS.
> > > > >>
> > > > >>Regards,
> > > > >>Andrei
> > > > >>
> > > > >>Valeri Fine wrote:
> > > > >>
> > > > >>>Hello team,
> > > > >>>
> > > > >>>What is the TGeoShape::Set(Get)ShapeBit property really mean?
> > > > >>>
> > > > >>>For example the list
> > > > >>>http://root.cern.ch/root/htmldoc/TGeoShape.html#TGeoShape:kGeoArb8
> > > > >>>      static const TGeoShape::EShapeType kGeoNoShape
> > > > >>>      static const TGeoShape::EShapeType kGeoBad
> > > > >>>      static const TGeoShape::EShapeType kGeoRSeg
> > > > >>>      static const TGeoShape::EShapeType kGeoPhiSeg
> > > > >>>      static const TGeoShape::EShapeType kGeoThetaSeg
> > > > >>>      static const TGeoShape::EShapeType kGeoVisX
> > > > >>>      static const TGeoShape::EShapeType kGeoVisY
> > > > >>>      static const TGeoShape::EShapeType kGeoVisZ
> > > > >>>      static const TGeoShape::EShapeType kGeoRunTimeShape
> > > > >>>      static const TGeoShape::EShapeType kGeoInvalidShape
> > > > >>>      static const TGeoShape::EShapeType kGeoTorus
> > > > >>>      static const TGeoShape::EShapeType kGeoBox
> > > > >>>      static const TGeoShape::EShapeType kGeoPara
> > > > >>>      static const TGeoShape::EShapeType kGeoSph
> > > > >>>      static const TGeoShape::EShapeType kGeoTube
> > > > >>>      static const TGeoShape::EShapeType kGeoTubeSeg
> > > > >>>      static const TGeoShape::EShapeType kGeoCone
> > > > >>>      static const TGeoShape::EShapeType kGeoConeSeg
> > > > >>>      static const TGeoShape::EShapeType kGeoPcon
> > > > >>>      static const TGeoShape::EShapeType kGeoPgon
> > > > >>>      static const TGeoShape::EShapeType kGeoArb8
> > > > >>>      static const TGeoShape::EShapeType kGeoEltu
> > > > >>>      static const TGeoShape::EShapeType kGeoTrap
> > > > >>>      static const TGeoShape::EShapeType kGeoCtub
> > > > >>>      static const TGeoShape::EShapeType kGeoTrd1
> > > > >>>      static const TGeoShape::EShapeType kGeoTrd2
> > > > >>>      static const TGeoShape::EShapeType kGeoComb
> > > > >>>      static const TGeoShape::EShapeType kGeoClosedShape
> > > > >>>
> > > > >>>
> > > > >>>suggests the TGeoTrap::GetShapeBit would contains kGeoTrap bit.
> > > > >>>However TGeoTrap:GetShapeBits returns "kGeoArb8"
> > > > >>>
> > > > >>>Is it a bug or undocumented feature? What the shape property stands
> > > > >
> > > > > for?
> > > > >
> > > > >>>Can some one elaborate a little bit to clarify when the shape type
> > > > >>>matches the shape bit?
> > > > >>>
> > > > >>>Thank you
> > > > >>>
> > > > >>>Valeri Fine
> > > > >>>Brookaven National Laboratiory
> > > > >>>P.O.Box 5000
> > > > >>>Upton, NY 11973-5000
> > > > >>>-----
> > > > >>>Phone: +1 631 344 7806
> > > > >>>Fax:   +1 631 344 4206
> > > > >>>E-mail: fine@bnl.gov
> > > > >>>
> > > > >>>
> > > > >>>
> > > > >>>
> > > > >
> > > > >
> > > > >
> > > >
> > >
> >
> 



This archive was generated by hypermail 2b29 : Sun Jan 02 2005 - 05:50:06 MET