Re: [ROOT] TGeoShape GetShapeBit policy?

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


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