----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118011/#review57435 -----------------------------------------------------------
Ship it! For m_geometry comparison, your second patch looks fine. It is deleted in ~GeoDataPlacemarkPrivate which is called from ~GeoDataFeature because the d-pointer is shared in the class hierarchy. Keeping code complexity low wasn't high on the priority list of geodata I'm afraid :-P - Dennis Nienhüser On May 5, 2014, 9:16 p.m., Cruceru Calin-Cristian wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/118011/ > ----------------------------------------------------------- > > (Updated May 5, 2014, 9:16 p.m.) > > > Review request for Marble. > > > Repository: marble > > > Description > ------- > > The previous implementation of operator== in GeoDataPlacemark was, as even > the documentation said, comparing the 2 private pointers of each instance of > the class. I thought that this implementation would be more appropriate, but > there are still some issues, especially caused by the m_geometry pointer: > - how should this be compared to the other's m_geometry? > - even though it is not regarded to what for I opened this bug review > request, we should think of that //FIXME comment within the GeoDataPlacemark( > const GeoDataPlacemark &other ) constructor because I think it will cause > problems when we will finish implementing operators== and we will start > testing on real .kml files. > - shouldn't m_geometry be deleted within the destructor, since its parent is > the current object? I see that it is deleted when set a new one geometry, but > not in destructor; maybe I didn't understand this correctly. > > > Diffs > ----- > > src/lib/marble/geodata/data/GeoDataPlacemark.h bd32b4c > src/lib/marble/geodata/data/GeoDataPlacemark.cpp 5df534d > > Diff: https://git.reviewboard.kde.org/r/118011/diff/ > > > Testing > ------- > > > Thanks, > > Cruceru Calin-Cristian > >
_______________________________________________ Marble-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/marble-devel
