[Kstars-devel] branches/work/kdeedu_kstars_htm/kstars/kstars/skycomponents

[Thomas Kabelmann]

Hi Jason,

you're completely right. The thing that worried me, is a cast to a sub type 
without checking, if the current parameter is from type SkyMapComposite. Now 
somebody might say that the constructor will allways called with variables of 
the right type. But in such a big complex project like KStars it will take 
not a long time that somebody calls it with incorrect types, so don't let us 
getting hackish. We need safety through interfaces that gives us the 
compiler. Anyway there should be a compiler warning for that cast (I hope C++ 
does it).

There are 2 possibilities to get that thing right:
- define all methods in SkyComponent as I suggested
- change signature of constructor as you suggested

The first proposal is pure theory and doesn't scale well with a growing number 
of methods. As I didn't looked at the code which components use this method, 
I just suggested to define this method for all components. If this method is 
just used by SkyMapComposite than your proposal is the better way as we don't 
pollute the main interface. If in the future other components, not derived 
from SkyMapComposite, but not all components need this functionality, we 
should define a new interface which the  corresponding components have to 
implement. Drawback is then that we have check the type at runtime, but 
better than casting of good luck. So currently I prefer your solution too.

Regards,
Thomas

Am Donnerstag, 9. August 2007 schrieben Sie:
> Hi Thomas,
>
> The c/c framework has been broken already many times already; why
> single out this one very minor example?  For example, there are many
> instances where we have found it convenient to make SkyMapComposite
> aware of its children in ways that seriously violate the strict
> definition of c/c.  This is okay IMO, because strict c/c isn't quite
> right for our use case.
>
> If the recasting of a SkyComponent to a SkyMapComposite is troubling,
> then we could simply change parent's type to SkyMapComposite:
>
> LineListComponent::LineListComponent( SkyMapComposite *parent )
>
>        : SkyComponent( parent ), LabelPosition( NoLabel ), Label(
>
> QString() )
> {
>        m_skyLabeler = parent->skyLabeler();
> }
>
> Note this technically breaks c/c, but it's okay because we only need
> LineListComponents that are direct children of SkyMapComposite.  If we
> ever need LineListComponents deeper in the hierarchy, then we can
> implement the dummy function as you suggest.
>
> regards,
> Jason