[Marble-devel] Review Request: Showing/editing the coords in bookmark editor

Friedrich W. H. Kossebau kossebau at kde.org
Wed Sep 14 20:22:50 UTC 2011



> On Sept. 14, 2011, 1:50 p.m., Torsten Rahn wrote:
> > src/lib/EditBookmarkDialog.h, line 66
> > <http://git.reviewboard.kde.org/r/102608/diff/1/?file=36108#file36108line66>
> >
> >     Usually we try to avoid method names starting with "on" and we try instead to describe what the method does. In this case this is a moot exercise, so we should make an exception here.

Agree.

Just sometimes the best description of such a method is: handleSignalX. Because that handling might involve all kind of actions which cannot be described in a single doFoo, rather doFooAndBarThenMoreFooButOnlyIf42.
See for some thoughts on it perhaps http://frinring.wordpress.com/2006/11/10/its-onbar-or-foo-not-slotfoo/ :)


- Friedrich W. H.


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102608/#review6502
-----------------------------------------------------------


On Sept. 13, 2011, 11:09 p.m., Friedrich W. H. Kossebau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102608/
> -----------------------------------------------------------
> 
> (Updated Sept. 13, 2011, 11:09 p.m.)
> 
> 
> Review request for Marble.
> 
> 
> Summary
> -------
> 
> This patchs adds the possibility to see and also edit also the coords of a bookmark in the bookmark editor.
> 
> Initially used a lineeddit and GeoDataCoordinates::toString, but GeoDataCoordinates::fromString cannot yet handle coords with secs it seems.
> So turned to use a LatLonEdit per coord.
> 
> Found LatLonEdit to be quite incomplete, so rewrote it to get it working for me.
> E.g. LatLonEdit::setDimension(...) was pretty broken.
> Also needed/wanted a double value for the secs editor, to enable more precision.
> No idea if 2 digits is a sane number of digits after the ".".
> 
> If LatLonEdit has to stay ABI compatible I will have to redo it then, as I changed the API
> (removed unused/useless dimensionChanged signal and changed some method signatures).
> 
> 
> Diffs
> -----
> 
>   src/lib/BookmarkManagerDialog.cpp 3f9db62 
>   src/lib/EditBookmarkDialog.h 4157726 
>   src/lib/EditBookmarkDialog.cpp 7e36ddc 
>   src/lib/EditBookmarkDialog.ui 9baf50d 
>   src/lib/LatLonEdit.h 4958805 
>   src/lib/LatLonEdit.cpp 758a167 
>   src/lib/LatLonEdit.ui 26774d3 
> 
> Diff: http://git.reviewboard.kde.org/r/102608/diff
> 
> 
> Testing
> -------
> 
> Added and viewed/edited several coords of geo bookmarks, worked all fine, same values shown after reloading or same places focussed if selected.
> 
> Also tested overflow behaviour of LatLonEdit for quite some border cases.
> 
> LatLonEdit widget still works fine in worldclock plasmoid.
> 
> 
> Thanks,
> 
> Friedrich W. H.
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/marble-devel/attachments/20110914/4b6aab49/attachment.html>


More information about the Marble-devel mailing list