<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/106276/">http://git.reviewboard.kde.org/r/106276/</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;">There are some welcome changes in this patch:
* ability to implement statistics by inheritance
* StatisticsCapability removal
However, there are some things I dislike:
* Meta::Track inheriting AbstractStatistics
* setAllStatistics() method (gosh!)
* Overusing the StatisticsProvider idea. I think that statistics are implemention detail of each collection. I even dislike the statistics_permanent and statistics_tag tables and I'd like to see them ditched. If someone wants to have statistics for tracks on filesystem, put them in tags. Otherwise put the tracks into Local Collection. I'd like if the SqlCollection would be the only thing that would actually require the SQL database.
What I'd like to do instead:
* StatisticsCapability is already ditched in my gsoc branch, with setPlaycount() and setFirst/LastPlayed() moved to EditCapability
* move also setRating(), setScore() into EditCapability
* refactor the whole capability concept (for tracks to start with) along with changing Meta::Track to be passed-by-value class with explicit data sharing. This would then mean that tracks could implement "Capabilities" (to be named better, perhaps Interfaces) by simply implementing the methods and then they could just "return this;" in relevant methods. Let me present you the suggestion in a couple of days.</pre>
<br />
<p>- Matěj</p>
<br />
<p>On August 30th, 2012, 12:03 p.m., Ralf Engels wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/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 Ralf Engels.</div>
<p style="color: grey;"><i>Updated Aug. 30, 2012, 12:03 p.m.</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;">This is try number three to get a consistent implementation of statistics handling.
This time with multiple inheritance.
This change will provide a complete implementation of statistics for a couple of collections.
That means that collections will get missing functional implementations e.g. setPlaycount.
For the future we will need a single sql table for all statistics and coverage for the ugly cases which are:
- PodcastMeta which currently is not covered.
- SqlMeta which uses it's own table and has problems with restoring statistics.
- A couple of collections that create tracks where the url is changed later on.
Oh, and it's 500 lines less code.
</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>src/core-impl/collections/audiocd/AudioCdMeta.h <span style="color: grey">(73ac547)</span></li>
<li>src/core-impl/collections/audiocd/AudioCdMeta.cpp <span style="color: grey">(d767125)</span></li>
<li>src/core-impl/collections/daap/DaapMeta.h <span style="color: grey">(5278b57)</span></li>
<li>src/core-impl/collections/daap/DaapMeta.cpp <span style="color: grey">(a2429d7)</span></li>
<li>src/core-impl/collections/db/sql/CapabilityDelegateImpl.cpp <span style="color: grey">(e344e0f)</span></li>
<li>src/core-impl/collections/db/sql/SqlMeta.h <span style="color: grey">(4450dab)</span></li>
<li>src/core-impl/collections/db/sql/SqlMeta.cpp <span style="color: grey">(c05e563)</span></li>
<li>src/core-impl/collections/db/sql/SqlRegistry.h <span style="color: grey">(f64b30c)</span></li>
<li>src/core-impl/collections/db/sql/SqlRegistry.cpp <span style="color: grey">(1444f94)</span></li>
<li>src/core-impl/collections/db/sql/SqlRegistry_p.h <span style="color: grey">(a7d4e26)</span></li>
<li>src/core-impl/collections/db/sql/SqlRegistry_p.cpp <span style="color: grey">(68cc871)</span></li>
<li>src/core-impl/collections/mediadevicecollection/MediaDeviceMeta.h <span style="color: grey">(2d0706e)</span></li>
<li>src/core-impl/collections/mediadevicecollection/MediaDeviceMeta.cpp <span style="color: grey">(3946b8f)</span></li>
<li>src/core-impl/collections/playdarcollection/PlaydarMeta.h <span style="color: grey">(93d1c2e)</span></li>
<li>src/core-impl/collections/playdarcollection/PlaydarMeta.cpp <span style="color: grey">(c9b07f3)</span></li>
<li>src/core-impl/collections/proxycollection/ProxyCollectionMeta.h <span style="color: grey">(e5d7266)</span></li>
<li>src/core-impl/collections/proxycollection/ProxyCollectionMeta.cpp <span style="color: grey">(091f5f6)</span></li>
<li>src/core-impl/collections/support/MemoryMeta.h <span style="color: grey">(675cf42)</span></li>
<li>src/core-impl/collections/upnpcollection/UpnpMeta.h <span style="color: grey">(feabc1e)</span></li>
<li>src/core-impl/collections/upnpcollection/UpnpMeta.cpp <span style="color: grey">(5ffee54)</span></li>
<li>src/core-impl/meta/file/File.h <span style="color: grey">(df96a2a)</span></li>
<li>src/core-impl/meta/file/File.cpp <span style="color: grey">(46f672b)</span></li>
<li>src/core-impl/meta/file/File_p.h <span style="color: grey">(f756074)</span></li>
<li>src/core-impl/meta/multi/MultiTrack.h <span style="color: grey">(63a8651)</span></li>
<li>src/core-impl/meta/proxy/MetaProxy.h <span style="color: grey">(0579d08)</span></li>
<li>src/core-impl/meta/proxy/MetaProxy.cpp <span style="color: grey">(51299f8)</span></li>
<li>src/core-impl/meta/stream/Stream.h <span style="color: grey">(8b64d89)</span></li>
<li>src/core-impl/meta/stream/Stream.cpp <span style="color: grey">(3eb54c7)</span></li>
<li>src/core-impl/meta/stream/Stream_p.h <span style="color: grey">(390109a)</span></li>
<li>src/core-impl/meta/timecode/TimecodeMeta.h <span style="color: grey">(ceb4d66)</span></li>
<li>src/core-impl/meta/timecode/TimecodeMeta.cpp <span style="color: grey">(f85476d)</span></li>
<li>src/core-impl/statistics/providers/tag/TagStatisticsProvider.h <span style="color: grey">(df79a00)</span></li>
<li>src/core-impl/statistics/providers/url/PermanentUrlStatisticsProvider.h <span style="color: grey">(7b6b3bb)</span></li>
<li>src/core-impl/statistics/providers/url/PermanentUrlStatisticsProvider.cpp <span style="color: grey">(9855954)</span></li>
<li>src/core/CMakeLists.txt <span style="color: grey">(29a215e)</span></li>
<li>src/core/capabilities/Capability.h <span style="color: grey">(8ec6d16)</span></li>
<li>src/core/capabilities/StatisticsCapability.h <span style="color: grey">(690ef12)</span></li>
<li>src/core/capabilities/StatisticsCapability.cpp <span style="color: grey">(34af392)</span></li>
<li>src/core/meta/Meta.h <span style="color: grey">(bf2184b)</span></li>
<li>src/core/meta/Meta.cpp <span style="color: grey">(df45908)</span></li>
<li>src/core/podcasts/PodcastMeta.h <span style="color: grey">(0cfaea1)</span></li>
<li>src/core/statistics/StatisticsProvider.h <span style="color: grey">(6f54912)</span></li>
<li>src/core/statistics/StatisticsProvider.cpp <span style="color: grey">(0b5a788)</span></li>
<li>src/databaseimporter/amarok14/FastForwardWorker.cpp <span style="color: grey">(56e319b)</span></li>
<li>src/databaseimporter/itunes/ITunesImporterWorker.cpp <span style="color: grey">(e5ad5e6)</span></li>
<li>src/services/ServiceMetaBase.h <span style="color: grey">(793eb9e)</span></li>
<li>src/services/ServiceMetaBase.cpp <span style="color: grey">(039146b)</span></li>
<li>src/services/magnatune/MagnatuneMeta.cpp <span style="color: grey">(523617b)</span></li>
<li>tests/core-impl/collections/db/sql/TestSqlScanManager.cpp <span style="color: grey">(c449216)</span></li>
<li>tests/core-impl/meta/file/TestMetaFileTrack.cpp <span style="color: grey">(7549a58)</span></li>
<li>tests/core/meta/TestMetaTrack.cpp <span style="color: grey">(0adb763)</span></li>
<li>tests/mocks/MetaMock.h <span style="color: grey">(b478c87)</span></li>
<li>tests/mocks/MockTrack.h <span style="color: grey">(57bc344)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/106276/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>