<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://svn.reviewboard.kde.org/r/6223/">http://svn.reviewboard.kde.org/r/6223/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On December 29th, 2010, 3:42 p.m., <b>Torsten Rahn</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://svn.reviewboard.kde.org/r/6223/diff/3/?file=43268#file43268line351" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdeedu/marble/src/lib/Planet.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">QString Planet::name( const QString& id )</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">351</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n">QStringList</span> <span class="n">Planet</span><span class="o">::</span><span class="n">planetList</span><span class="p">()</span><span class="ew"> </span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The implementation should be closer in sync with the one of the name method. Currently it would easily be possible to accidently have different string ids in both implementations, possibly contradicting each other. This makes maintenance harder and the implementation more prone to errors.</pre>
</blockquote>
<p>On December 29th, 2010, 4:27 p.m., <b>den</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Would this not occur with the method
Planet::Planet( const QString& id )
: d( new PlanetPrivate )
as well. Also could you suggest how I could achieve this because a list of variables of viable planet IDs seems a bad way to do it. </pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Yes, the danger is already in the current code. So ideally we'd need some kind of registration mechanism in the future for the Planet class.
So if you come up with some nice idea how to solve this (which might result in a new task maybe ;) don't hesitate to let us know :)
I have marked the task as complete. </pre>
<br />
<p>- Torsten</p>
<br />
<p>On December 29th, 2010, 1:37 p.m., den wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://svn.reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for marble.</div>
<div>By den.</div>
<p style="color: grey;"><i>Updated 2010-12-29 13:37:30</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Google Code In Task - http://socghop.appspot.com/gci/task/show/google/gci2010/kde/t129277012002
Adds a planet option combobox to the map creation wizard page.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/KDE/kdeedu/marble/src/lib/MapWizard.h <span style="color: grey">(1210090)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MapWizard.cpp <span style="color: grey">(1210090)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MapWizard.ui <span style="color: grey">(1210090)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/Planet.h <span style="color: grey">(1210090)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/Planet.cpp <span style="color: grey">(1210090)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/6223/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Screenshots </h1>
<div>
<a href="http://svn.reviewboard.kde.org/r/6223/s/591/"><img src="http://svn.reviewboard.kde.org/media/uploaded/images/2010/12/27/Screen_shot_2010-12-27_at_21.20.51_400x100.png" style="border: 1px black solid;" alt="" /></a>
</div>
</td>
</tr>
</table>
</div>
</body>
</html>