refactor: move internalpayerbackend out of config.cc to src/audio

.github/workflows/PR-check-cmake.yml

@@ -67,7 +67,7 @@ jobs:
           libzim \
           qt
 
-          wget ftp://ftp.sra.co.jp/pub/misc/eb/eb-4.4.3.tar.bz2
+          wget https://github.com/mistydemeo/eb/releases/download/v4.4.3/eb-4.4.3.tar.bz2
           tar xvjf eb-4.4.3.tar.bz2
           cd eb-4.4.3 && ./configure && make -j 8 && sudo make install && cd ..
 

src/audio/audioplayerfactory.cc

@@ -3,34 +3,39 @@
 
 #include <QScopedPointer>
 #include <QObject>
+#include <utility>
 #include "audioplayerfactory.hh"
 #include "ffmpegaudioplayer.hh"
 #include "multimediaaudioplayer.hh"
 #include "externalaudioplayer.hh"
 #include "gddebug.hh"
 
-AudioPlayerFactory::AudioPlayerFactory( Config::Preferences const & p ):
-  useInternalPlayer( p.useInternalPlayer ),
-  internalPlayerBackend( p.internalPlayerBackend ),
-  audioPlaybackProgram( p.audioPlaybackProgram )
+AudioPlayerFactory::AudioPlayerFactory( bool useInternalPlayer,
+                                        InternalPlayerBackend internalPlayerBackend,
+                                        QString audioPlaybackProgram ):
+  useInternalPlayer( useInternalPlayer ),
+  internalPlayerBackend( std::move( internalPlayerBackend ) ),
+  audioPlaybackProgram( std::move( audioPlaybackProgram ) )
 {
   reset();
 }
 
-void AudioPlayerFactory::setPreferences( Config::Preferences const & p )
+void AudioPlayerFactory::setPreferences( bool new_useInternalPlayer,
+                                         const InternalPlayerBackend & new_internalPlayerBackend,
+                                         const QString & new_audioPlaybackProgram )
 {
-  if ( p.useInternalPlayer != useInternalPlayer ) {
-    useInternalPlayer     = p.useInternalPlayer;
-    internalPlayerBackend = p.internalPlayerBackend;
-    audioPlaybackProgram  = p.audioPlaybackProgram;
+  if ( useInternalPlayer != new_useInternalPlayer ) {
+    useInternalPlayer     = new_useInternalPlayer;
+    internalPlayerBackend = new_internalPlayerBackend;
+    audioPlaybackProgram  = new_audioPlaybackProgram;
     reset();
   }
-  else if ( useInternalPlayer && p.internalPlayerBackend != internalPlayerBackend ) {
-    internalPlayerBackend = p.internalPlayerBackend;
+  else if ( useInternalPlayer && internalPlayerBackend != new_internalPlayerBackend ) {
+    internalPlayerBackend = new_internalPlayerBackend;
     reset();
   }
-  else if ( !useInternalPlayer && p.audioPlaybackProgram != audioPlaybackProgram ) {
-    audioPlaybackProgram                       = p.audioPlaybackProgram;
+  else if ( !useInternalPlayer && new_audioPlaybackProgram != audioPlaybackProgram ) {
+    audioPlaybackProgram                       = new_audioPlaybackProgram;
     ExternalAudioPlayer * const externalPlayer = qobject_cast< ExternalAudioPlayer * >( playerPtr.data() );
     if ( externalPlayer ) {
       setAudioPlaybackProgram( *externalPlayer );
@@ -50,7 +55,7 @@ void AudioPlayerFactory::reset()
     // another object of the same type.
 
 #ifdef MAKE_FFMPEG_PLAYER
-    Q_ASSERT( Config::InternalPlayerBackend::defaultBackend().isFfmpeg()
+    Q_ASSERT( InternalPlayerBackend::defaultBackend().isFfmpeg()
               && "Adjust the code below after changing the default backend." );
 
     if ( !internalPlayerBackend.isQtmultimedia() ) {

src/audio/audioplayerfactory.hh

@@ -4,7 +4,7 @@
 #pragma once
 
 #include "audioplayerinterface.hh"
-#include "config.hh"
+#include "internalplayerbackend.hh"
 
 class ExternalAudioPlayer;
 
@@ -13,8 +13,12 @@ class AudioPlayerFactory
   Q_DISABLE_COPY( AudioPlayerFactory )
 
 public:
-  explicit AudioPlayerFactory( Config::Preferences const & );
-  void setPreferences( Config::Preferences const & );
+  explicit AudioPlayerFactory( bool useInternalPlayer,
+                               InternalPlayerBackend internalPlayerBackend,
+                               QString audioPlaybackProgram );
+  void setPreferences( bool new_useInternalPlayer,
+                       const InternalPlayerBackend & new_internalPlayerBackend,
+                       const QString & new_audioPlaybackProgram );
   /// The returned reference to a smart pointer is valid as long as this object
   /// exists. The pointer to the owned AudioPlayerInterface may change after the
   /// call to setPreferences(), but it is guaranteed to never be null.
@@ -28,7 +32,7 @@ private:
   void setAudioPlaybackProgram( ExternalAudioPlayer & externalPlayer );
 
   bool useInternalPlayer;
-  Config::InternalPlayerBackend internalPlayerBackend;
+  InternalPlayerBackend internalPlayerBackend;
   QString audioPlaybackProgram;
   AudioPlayerPtr playerPtr;
 };

src/audio/internalplayerbackend.cc

@@ -0,0 +1,51 @@
+#include "internalplayerbackend.hh"
+
+bool InternalPlayerBackend::anyAvailable()
+{
+#if defined( MAKE_FFMPEG_PLAYER ) || defined( MAKE_QTMULTIMEDIA_PLAYER )
+  return true;
+#else
+  return false;
+#endif
+}
+
+InternalPlayerBackend InternalPlayerBackend::defaultBackend()
+{
+#if defined( MAKE_FFMPEG_PLAYER )
+  return ffmpeg();
+#elif defined( MAKE_QTMULTIMEDIA_PLAYER )
+  return qtmultimedia();
+#else
+  return InternalPlayerBackend( QString() );
+#endif
+}
+
+QStringList InternalPlayerBackend::nameList()
+{
+  QStringList result;
+#ifdef MAKE_FFMPEG_PLAYER
+  result.push_back( ffmpeg().uiName() );
+#endif
+#ifdef MAKE_QTMULTIMEDIA_PLAYER
+  result.push_back( qtmultimedia().uiName() );
+#endif
+  return result;
+}
+
+bool InternalPlayerBackend::isFfmpeg() const
+{
+#ifdef MAKE_FFMPEG_PLAYER
+  return *this == ffmpeg();
+#else
+  return false;
+#endif
+}
+
+bool InternalPlayerBackend::isQtmultimedia() const
+{
+#ifdef MAKE_QTMULTIMEDIA_PLAYER
+  return *this == qtmultimedia();
+#else
+  return false;
+#endif
+}

src/audio/internalplayerbackend.hh

@@ -0,0 +1,61 @@
+#pragma once
+#include <QStringList>
+
+/// Overly engineered dummy/helper/wrapper "backend", which is not, to manage backends.
+class InternalPlayerBackend
+{
+public:
+  /// Returns true if at least one backend is available.
+  static bool anyAvailable();
+  /// Returns the default backend or null backend if none is available.
+  static InternalPlayerBackend defaultBackend();
+  /// Returns the name list of supported backends.
+  static QStringList nameList();
+
+  /// Returns true if built with FFmpeg player support and the name matches.
+  bool isFfmpeg() const;
+  /// Returns true if built with Qt Multimedia player support and the name matches.
+  bool isQtmultimedia() const;
+
+  QString const & uiName() const
+  {
+    return name;
+  }
+
+  void setUiName( QString const & name_ )
+  {
+    name = name_;
+  }
+
+  bool operator==( InternalPlayerBackend const & other ) const
+  {
+    return name == other.name;
+  }
+
+  bool operator!=( InternalPlayerBackend const & other ) const
+  {
+    return !operator==( other );
+  }
+
+private:
+#ifdef MAKE_FFMPEG_PLAYER
+  static InternalPlayerBackend ffmpeg()
+  {
+    return InternalPlayerBackend( "FFmpeg" );
+  }
+#endif
+
+#ifdef MAKE_QTMULTIMEDIA_PLAYER
+  static InternalPlayerBackend qtmultimedia()
+  {
+    return InternalPlayerBackend( "Qt Multimedia" );
+  }
+#endif
+
+  explicit InternalPlayerBackend( QString const & name_ ):
+    name( name_ )
+  {
+  }
+
+  QString name;
+};

src/config.cc

@@ -119,57 +119,6 @@ QKeySequence HotKey::toKeySequence() const
   ;
 }
 
-
-bool InternalPlayerBackend::anyAvailable()
-{
-#if defined( MAKE_FFMPEG_PLAYER ) || defined( MAKE_QTMULTIMEDIA_PLAYER )
-  return true;
-#else
-  return false;
-#endif
-}
-
-InternalPlayerBackend InternalPlayerBackend::defaultBackend()
-{
-#if defined( MAKE_FFMPEG_PLAYER )
-  return ffmpeg();
-#elif defined( MAKE_QTMULTIMEDIA_PLAYER )
-  return qtmultimedia();
-#else
-  return InternalPlayerBackend( QString() );
-#endif
-}
-
-QStringList InternalPlayerBackend::nameList()
-{
-  QStringList result;
-#ifdef MAKE_FFMPEG_PLAYER
-  result.push_back( ffmpeg().uiName() );
-#endif
-#ifdef MAKE_QTMULTIMEDIA_PLAYER
-  result.push_back( qtmultimedia().uiName() );
-#endif
-  return result;
-}
-
-bool InternalPlayerBackend::isFfmpeg() const
-{
-#ifdef MAKE_FFMPEG_PLAYER
-  return *this == ffmpeg();
-#else
-  return false;
-#endif
-}
-
-bool InternalPlayerBackend::isQtmultimedia() const
-{
-#ifdef MAKE_QTMULTIMEDIA_PLAYER
-  return *this == qtmultimedia();
-#else
-  return false;
-#endif
-}
-
 QString Preferences::sanitizeInputPhrase( QString const & inputWord ) const
 {
   QString result = inputWord;

src/config.hh

@@ -3,19 +3,20 @@
 
 #pragma once
 
-#include <QObject>
-#include <QList>
-#include <QString>
-#include <QSize>
-#include <QDateTime>
-#include <QKeySequence>
-#include <QSet>
-#include <QMetaType>
+#include "audio/internalplayerbackend.hh"
 #include "ex.hh"
+#include <QDateTime>
 #include <QDomDocument>
+#include <QKeySequence>
+#include <QList>
 #include <QLocale>
-#include <optional>
+#include <QMetaType>
+#include <QObject>
+#include <QSet>
+#include <QSize>
+#include <QString>
 #include <QThread>
+#include <optional>
 
 /// Special group IDs
 enum GroupId : unsigned {
@@ -269,66 +270,6 @@ struct CustomFonts
   }
 };
 
-/// This class encapsulates supported backend preprocessor logic,
-/// discourages duplicating backend names in code, which is error-prone.
-class InternalPlayerBackend
-{
-public:
-  /// Returns true if at least one backend is available.
-  static bool anyAvailable();
-  /// Returns the default backend or null backend if none is available.
-  static InternalPlayerBackend defaultBackend();
-  /// Returns the name list of supported backends.
-  static QStringList nameList();
-
-  /// Returns true if built with FFmpeg player support and the name matches.
-  bool isFfmpeg() const;
-  /// Returns true if built with Qt Multimedia player support and the name matches.
-  bool isQtmultimedia() const;
-
-  QString const & uiName() const
-  {
-    return name;
-  }
-
-  void setUiName( QString const & name_ )
-  {
-    name = name_;
-  }
-
-  bool operator==( InternalPlayerBackend const & other ) const
-  {
-    return name == other.name;
-  }
-
-  bool operator!=( InternalPlayerBackend const & other ) const
-  {
-    return !operator==( other );
-  }
-
-private:
-#ifdef MAKE_FFMPEG_PLAYER
-  static InternalPlayerBackend ffmpeg()
-  {
-    return InternalPlayerBackend( "FFmpeg" );
-  }
-#endif
-
-#ifdef MAKE_QTMULTIMEDIA_PLAYER
-  static InternalPlayerBackend qtmultimedia()
-  {
-    return InternalPlayerBackend( "Qt Multimedia" );
-  }
-#endif
-
-  explicit InternalPlayerBackend( QString const & name_ ):
-    name( name_ )
-  {
-  }
-
-  QString name;
-};
-
 /// Various user preferences
 struct Preferences
 {

src/ui/mainwindow.cc

@@ -167,7 +167,8 @@ MainWindow::MainWindow( Config::Class & cfg_ ):
                  cfg.preferences.disallowContentFromOtherSites,
                  cfg.preferences.hideGoldenDictHeader ),
   dictNetMgr( this ),
-  audioPlayerFactory( cfg.preferences ),
+  audioPlayerFactory(
+    cfg.preferences.useInternalPlayer, cfg.preferences.internalPlayerBackend, cfg.preferences.audioPlaybackProgram ),
   wordFinder( this ),
   wordListSelChanged( false ),
   wasMaximized( false ),
@@ -2356,7 +2357,9 @@ void MainWindow::editPreferences()
 #endif
     }
 
-    audioPlayerFactory.setPreferences( cfg.preferences );
+    audioPlayerFactory.setPreferences( cfg.preferences.useInternalPlayer,
+                                       cfg.preferences.internalPlayerBackend,
+                                       cfg.preferences.audioPlaybackProgram );
 
     trayIconUpdateOrInit();
     applyProxySettings();

src/ui/preferences.cc

@@ -294,7 +294,7 @@ Preferences::Preferences( QWidget * parent, Config::Class & cfg_ ):
   ui.pronounceOnLoadMain->setChecked( p.pronounceOnLoadMain );
   ui.pronounceOnLoadPopup->setChecked( p.pronounceOnLoadPopup );
 
-  ui.internalPlayerBackend->addItems( Config::InternalPlayerBackend::nameList() );
+  ui.internalPlayerBackend->addItems( InternalPlayerBackend::nameList() );
 
   // Make sure that exactly one radio button in the group is checked and that
   // on_useExternalPlayer_toggled() is called.
@@ -306,7 +306,7 @@ Preferences::Preferences( QWidget * parent, Config::Class & cfg_ ):
 
     int index = ui.internalPlayerBackend->findText( p.internalPlayerBackend.uiName() );
     if ( index < 0 ) { // The specified backend is unavailable.
-      index = ui.internalPlayerBackend->findText( Config::InternalPlayerBackend::defaultBackend().uiName() );
+      index = ui.internalPlayerBackend->findText( InternalPlayerBackend::defaultBackend().uiName() );
     }
     Q_ASSERT( index >= 0 && "Logic error: the default backend must be present in the backend name list." );
     ui.internalPlayerBackend->setCurrentIndex( index );