extragear/multimedia/amarok/src

[Maximilian Kossick]

Neither EngineController nor MainWindow is a good place to put this
signal. I certainly would not look in either class when trying to
figure out how to love track. We are loving tracks, so the method
should be associated relatively closely with Meta::Track (is there any
particular reason that we only allow loving of the current track
anyway?). It would make sense to put the love() method into
Meta::Track if all tracks have this capability, but I'd put it into a
capability instead (and then provide a default implementation of that
capability for all tracks to use...one can always subclass the default
implementation to add specific behaviour). Putting love into a
capability helps us to keep the interface of Meta::Track as simple as
possible. We'd have to add that capability to all tracks (and access
that capability wherever we want to love a track), but that's still
the best solution in my opinion because it does not mix
responsibilities .

It certainly makes sense to rely on the last.fm service for all
last.fm web service calls because the last.fm client code is there.
but we need some kind of adapter or proxy in libamarok that is called
whenever another part of amarok wants to interact with last.fm, and
then forwards those calls to the last.fm service (or the last.fm
service pulls the requests, but that class in libamarok must be able
to make sure that the last.fm service actually exists).

Max

On Tue, Apr 22, 2008 at 5:00 AM, Dan Meltzer
<hydrogen at notyetimplemented.com> wrote:
> SVN commit 799706 by dmeltzer:
>
>  Move the method to MainWindow for now... we may want to think about making loved a property in Meta::Track to allow for service specific implementations as well.  Thoughts Max?
>  CCMAIL: maximilian.kossick at gmail.com
>
>
>   M  +0 -7      EngineController.cpp
>   M  +0 -2      EngineController.h
>   M  +8 -1      MainWindow.cpp
>   M  +4 -0      MainWindow.h
>   M  +1 -1      amarokcore/amarokdbushandler.cpp
>   M  +3 -2      servicebrowser/lastfm/ScrobblerAdapter.cpp
>
>
>  --- trunk/extragear/multimedia/amarok/src/EngineController.cpp #799705:799706
>  @@ -318,13 +318,6 @@
>   }
>
>   void
>  -EngineController::loveTrack()
>  -{
>  -    if( m_currentTrack )
>  -        emit loveTrack( m_currentTrack );
>  -}
>  -
>  -void
>   EngineController::seek( int ms ) //SLOT
>   {
>      if( m_media->isSeekable() )
>  --- trunk/extragear/multimedia/amarok/src/EngineController.h #799705:799706
>  @@ -78,7 +78,6 @@
>      void pause();
>      void stop( bool forceInstant = false );
>      void playPause(); //pauses if playing, plays if paused or stopped
>  -    void loveTrack();
>
>      void seek( int ms );
>      void seekRelative( int ms );
>  @@ -94,7 +93,6 @@
>   signals:
>      void statusText( const QString& );
>      void trackFinished();
>  -    void loveTrack( Meta::TrackPtr );
>
>   protected:
>      void playUrl( const KUrl &url, uint offset );
>  --- trunk/extragear/multimedia/amarok/src/MainWindow.cpp #799705:799706
>  @@ -764,6 +764,13 @@
>   #endif
>   }
>
>  +void
>  +MainWindow::loveTrack()
>  +{
>  +    Meta::TrackPtr cTrack = The::engineController()->currentTrack();
>  +    if( cTrack )
>  +        emit loveTrack( cTrack );
>  +}
>   void MainWindow::activate()
>   {
>   #ifdef Q_WS_X11
>  @@ -1010,7 +1017,7 @@
>      action->setObjectName( "loveTrack" );
>      action->setGlobalShortcut( KShortcut( Qt::META + Qt::Key_L ) );
>      ac->addAction( "loveTrack", action );
>  -    connect( action, SIGNAL(triggered()), ec, SLOT(loveTrack()) );
>  +    connect( action, SIGNAL(triggered()), SLOT(loveTrack()) );
>
>      action = new KAction( i18n( "Rate Current Track: 1" ), this );
>      action->setObjectName( "rate1" );
>  --- trunk/extragear/multimedia/amarok/src/MainWindow.h #799705:799706
>  @@ -65,8 +65,12 @@
>          SideBar *sideBar() const { return m_browsers; }
>          void deleteBrowsers();
>
>  +    signals:
>  +        void loveTrack( Meta::TrackPtr );
>  +
>      public slots:
>          void showHide();
>  +        void loveTrack();
>
>      private slots:
>          void slotShrinkBrowsers( int index ) const;
>  --- trunk/extragear/multimedia/amarok/src/amarokcore/amarokdbushandler.cpp #799705:799706
>  @@ -345,7 +345,7 @@
>
>      void DbusPlayerHandler::loveTrack()
>      {
>  -        The::engineController()->loveTrack();
>  +        MainWindow::self()->loveTrack();
>      }
>
>      void DbusPlayerHandler::mediaDeviceMount()
>  --- trunk/extragear/multimedia/amarok/src/servicebrowser/lastfm/ScrobblerAdapter.cpp #799705:799706
>  @@ -14,9 +14,10 @@
>   #define DEBUG_PREFIX "lastfm"
>
>   #include "ScrobblerAdapter.h"
>  -#include "EngineController.h"
>   #include "Amarok.h"
>   #include "debug.h"
>  +#include "EngineController.h"
>  +#include "MainWindow.h"
>   #include "MetaConstants.h"
>   #include "meta/LastFmMeta.h"
>   #include "TheInstances.h"
>  @@ -32,7 +33,7 @@
>      resetVariables();
>
>      connect( m_manager, SIGNAL( status( int, QVariant ) ), this, SLOT( statusChanged( int, QVariant ) ) );
>  -    connect( The::engineController(), SIGNAL(loveTrack(Meta::TrackPtr)), SLOT(slotTrackLoved(Meta::TrackPtr)));
>  +    connect( MainWindow::self(), SIGNAL(loveTrack(Meta::TrackPtr)), SLOT(slotTrackLoved(Meta::TrackPtr)));
>
>      Scrobbler::Init init;
>      init.username = username;
>