Re: [ROOT] TGeoShape GetShapeBit policy?

From: Victor Perevoztchikov (perev@bnl.gov)
Date: Mon Feb 23 2004 - 18:19:50 MET


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