[Kstars-devel] Refined patch for wishlist 178232: List all objects within a rectangle

[Abhijit Apte]

Hi Alexey,

Thanks for your comments, pl. find my comments inlined.

1. Please change signature of findObjectInArea to
> findObjectInArea(const SkyPoint&, const SkyPoint&)
> It will make obvious to caller that points are not going to be changed.
>
> This may require copying but it's not a problem.


> 2. I'm not sure that SkyMapComposite::skyRegion is eligible for separate
> function. It's as simple as
>
> SkyPoint p3( p1.ra(), p2.dec() );
> SkyPoint p4( p2.ra(), p1.dec() ); // Or other way around
> QList<SkyPoint*> list;
> // "<<" is overloaded for QList<T>
> list << &p1 << &p3 << &p2 << &p4;
>

Two reasons:
1) Have added the function skyRegion() as a separate function because later
on we can scale it up to support a user-defined region.. Then one has to do,
is to write overloaded functions for skyRegion(). Say user wants to select
from a circular region or a polygon for instance. I think it's fine to add
this skyRegion() function for now.

2) The problem with making SkyPoint p1 and p2 const:
While filling up the list with operator<<, it'd become a problem since QList
is instantiated with SkyPoint* whereas the arguments are of type "const
SkyPoint*".
/home/kde-devel/kde/src/KDE/refactor/kdeedu/kstars/kstars/skycomponents/skymapcomposite.cpp:
In member function ‘QList<SkyObject*>
SkyMapComposite::findObjectsInArea(const SkyPoint&, const SkyPoint&)’:
/home/kde-devel/kde/src/KDE/refactor/kdeedu/kstars/kstars/skycomponents/skymapcomposite.cpp:348:
error: invalid conversion from ‘const SkyPoint*’ to ‘SkyPoint*’
/home/kde-devel/kde/src/KDE/refactor/kdeedu/kstars/kstars/skycomponents/skymapcomposite.cpp:348:
error:   initializing argument 1 of ‘QVector<T>&
QVector<T>::operator<<(const T&) [with T = SkyPoint*]’


> Please note that there is no need to create temporary variables. Also `dms'
> has size of double since it's double in fancy wrapping.
>
> Sure thanks for pointing out. I'll remove those dms temporaries. They add
up to memory.


> 3. It doesn't seem very reasonable to leave out nameless stars. They are
> vast
> majority after all. This will require to set up some magnitude cut-off to
> remove faint stars at low zoom.
>
> This is difficult task and separate from object finding so it's fine as it
> is by
> now.
>

I agree.


> 4. If object doesn't visible it shouldn't get listed. You can use
> `selected()'
> to determine whether component visible.
>

You mean the objects below horizon?
I didn't quite understand this -- I noticed each component has this
selected() function. Did you mean the following because the component views
can be toggled from toolbar?? If you are referring to it, it's a good catch
! :)
if( m_Stars->selected() ) m_Stars->findObjectsInArea(p1,p2)
if( m_DeepSky->selected() ) m_DeepSky->findObjectsInArea(p1,p2)

Rgds
-Abhi
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kstars-devel/attachments/20100205/4c6fa4c8/attachment.htm