<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://git.reviewboard.kde.org/r/100168/">http://git.reviewboard.kde.org/r/100168/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">You got me wrong. I meant you should check albums and artists for existence, not for empty name. Because ArtistPtr/AlbumPtr can point to nowhere in case of NULL artist/album in DB.</pre>
<br />
<div>
<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://git.reviewboard.kde.org/r/100168/diff/2/?file=4282#file4282line310" style="color: black; font-weight: bold; text-decoration: underline;">src/playlistmanager/sql/SqlUserPlaylistProvider.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">SqlUserPlaylistProvider::generatePlaylistName( const Meta::TrackList &tracks, QString *name )</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">310</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QString</span> <span class="n">artist</span> <span class="o">=</span> <span class="n">tracks</span><span class="p">.</span><span class="n">first</span><span class="p">()</span><span class="o">-></span><span class="n">artist</span><span class="p">()</span><span class="o">-></span><span class="n">prettyName</span><span class="p">().</span><span class="n">trimmed</span><span class="p">();</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">It should be like:
QString artist = tracks.first()->artist() ? tracks.first()->artist()->prettyName() : QString();
here</pre>
</div>
<br />
<div>
<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://git.reviewboard.kde.org/r/100168/diff/2/?file=4282#file4282line315" style="color: black; font-weight: bold; text-decoration: underline;">src/playlistmanager/sql/SqlUserPlaylistProvider.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">SqlUserPlaylistProvider::generatePlaylistName( const Meta::TrackList &tracks, QString *name )</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">315</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span><span class="p">(</span> <span class="n">artist</span> <span class="o">!=</span> <span class="n">track</span><span class="o">-></span><span class="n">artist</span><span class="p">()</span><span class="o">-></span><span class="n">prettyName</span><span class="p">().</span><span class="n">trimmed</span><span class="p">()</span> <span class="p">)</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">And
if( !track->artist() || artist != track->artist()->prettyName() )
here.</pre>
</div>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">It can be improved a bit by using album artist in case of singleAlbum.
And since all Artist/Album pointers are shared, you don't need to compare Its names, you can compare pointers. It will reduce overhead from strings comparisons and you won't need to check existence of artists/albums 'till you try to get Its names in 325-332 lines.</pre>
<p>- Sergey</p>
<br />
<p>On November 25th, 2010, 1:01 a.m., Dennis Francis wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.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 Amarok.</div>
<div>By Dennis Francis.</div>
<p style="color: grey;"><i>Updated 2010-11-25 01:01:23</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;">The playlist name is generated as per the rules :
if ( singleArtist && singleAlbum )
playlistName = artist + " - " + album + ' - ' + shortDate
else if ( !singleArtist && singleAlbum )
playlistName = "Various" + " - " + album + ' - ' + shortDate
else if ( singleArtist && !singleAlbum )
playlistName = artist + " - " + "Various" + shortDate
else
playlistName = "Various" + " - " + shortDate
If the there are no tracks, the playlist name is set to "Empty Playlist - " + shortDate
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </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;">Tested different possibilities. It is working fine for me.
Please suggest better alternatives ( if any ) to the shortDate addition. Without the shortDate, playlist database lookup would be necessary to
calculate a unique name.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=185397">185397</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/playlistmanager/sql/SqlUserPlaylistProvider.h <span style="color: grey">(3a5a62e)</span></li>
<li>src/playlistmanager/sql/SqlUserPlaylistProvider.cpp <span style="color: grey">(d2ae5b9)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/100168/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>