----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124232/#review82010 -----------------------------------------------------------
src/lib/marble/geodata/data/GeoDataData.h (line 50) <https://git.reviewboard.kde.org/r/124232/#comment56358> Breaks const correntness. We need both QVariant& GeoDataData::valueRef() const QVariant& GeoDataData::valueRef() const This will likely cause troubles further down in the code. src/lib/marble/geodata/data/GeoDataPlacemark.h (line 121) <https://git.reviewboard.kde.org/r/124232/#comment56356> src/lib/marble/geodata/data/GeoDataPlacemark.cpp (line 177) <https://git.reviewboard.kde.org/r/124232/#comment56360> For short methods it's ok to have some code duplication between a method and its const overload. Given the length here though I'd prefer an implementation along the lines of https://stackoverflow.com/questions/4589622/simplifying-const-overloading src/lib/marble/geodata/data/GeoDataPlacemark.cpp (line 198) <https://git.reviewboard.kde.org/r/124232/#comment56357> With the code change above (const correctness) this will probably not work anymore. One workaround would be a mutable extended data, but that seems a bit far reaching. Probably a const_cast is the least intrusive. - Dennis Nienhüser On July 2, 2015, 4:06 p.m., Marius Stanciu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/124232/ > ----------------------------------------------------------- > > (Updated July 2, 2015, 4:06 p.m.) > > > Review request for Marble. > > > Repository: marble > > > Description > ------- > > Adding this to the GeoDataPlacemark might not be really the best idea, but > this makes accessing the osmdata object within a placemark's extended data > really easy, and very similar to the other accessors. > > > Diffs > ----- > > src/lib/marble/geodata/data/GeoDataData.h 491cb61 > src/lib/marble/geodata/data/GeoDataData.cpp e565c2c > src/lib/marble/geodata/data/GeoDataPlacemark.h 726befb > src/lib/marble/geodata/data/GeoDataPlacemark.cpp 7af4f9a > > Diff: https://git.reviewboard.kde.org/r/124232/diff/ > > > Testing > ------- > > > Thanks, > > Marius Stanciu > >
_______________________________________________ Marble-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/marble-devel
