[Kstars-devel] AuxData and QStrings in unnamed star objects

[James Bowlin]

IIRC, the current (incorrect) implementation of AuxInfo in unnamed stars 
was a temporary placeholder.  When a star object gets destroyed we do 
NOT want to also destroy the AuxData (which the user entered).  The 
reason for this is that we are swapping stars in and out of memory, and 
this should be totally transparent to the user.

The plan was to add a layer of indirection for the AuxData field
so the actual pointers to AuxData objects would reside as values
in a QHash that would live at least as long as the KStars program lived.
Eventually it might even get saved to disk (does it already?) to live
between invocations of KStars.  The keys in the hash would be unique 
identifiers in each star object so when a star is deleted and created 
it still would get the same original AuxInfo object.

IMO the correct thing to do is to now implement the hash instead of 
trying to destroy AuxData correctly.  IIRC hashing AuxInfo objects was 
the one of the next things on the TODO list when we ran out of time.  


Also, one way to deal with the QStrings in unnamed star objects is to 
make sure we use pointers to QStrings instead of QString objects or 
references.  Then we don't have to worry about freeing QStrings inside
of unnamed stars.  I think (but I'm not at all sure) that this might
also work for references.  I *think* that references are internally
(to C++) just like pointers.  So if all of the unnamed stars are 
referencing the same QStrings, then again we don't have to worry about
freeing QStrings when deleting unnamed stars.  In fact we might have
relied on a trick like this.  It is possible that the QStrings are okay
(although ugly) and the only thing that needs to be fixed is AuxInfo.
Although if this is the case, we certainly need to comment the code to
explain what is going on.


When the star swapping code was being written, we did discuss using 
placement new instead of memcpy.  ISTM that it did pretty much the
same thing as memcpy with the same benefits and disadvantages.  The
memcpy code was already in place and we were pressed for time so IIRC,
we did not circle back and try using placement new instead of memcpy.
As long as it does not slow things down, I think placement new is better
than memcpy because it is more C++-like.

We went to great lengths to make the star swapping code as fast as 
possible.  I'm very glad someone has taken an interest in cleaning
it up.  There is no question that we must fix bugs, especially bugs
that could cause crashes.  But I think it is important to try to keep
that code as fast as possible (without crashes) even at the expense
of appearing un-C++-like.  IMO as fast as possible then as C++ as
possible is the ideal.


-- 
Peace, James