Fixes from code review for PR #7037

src/devices/cddadevice.cpp

@@ -22,7 +22,7 @@
 #include "library/librarybackend.h"
 #include "library/librarymodel.h"
 
-const int CddaDevice::kDiscChangePollingIntervalMs = 500;
+const int kDiscChangePollingIntervalMs = 500;
 
 CddaDevice::CddaDevice(const QUrl& url, DeviceLister* lister,
                        const QString& unique_id, DeviceManager* manager,
@@ -68,7 +68,7 @@ bool CddaDevice::IsValid() const { return (cdio_ != nullptr); }
 
 void CddaDevice::WatchForDiscChanges(bool watch) {
   if (watch && !disc_changed_timer_.isActive())
-    disc_changed_timer_.start(CddaDevice::kDiscChangePollingIntervalMs);
+    disc_changed_timer_.start(kDiscChangePollingIntervalMs);
   else if (!watch && disc_changed_timer_.isActive())
     disc_changed_timer_.stop();
 }

src/devices/cddadevice.h

@@ -53,7 +53,6 @@ class CddaDevice : public ConnectedDevice {
   bool IsValid() const;
   void WatchForDiscChanges(bool watch);
 
-  static const int kDiscChangePollingIntervalMs;
   static QStringList url_schemes() { return QStringList() << "cdda"; }
 
   // QUrl interprets a single number as an ip address, so the QString cdda://1

src/ripper/ripcddialog.cpp

@@ -54,15 +54,16 @@ const int kTrackDurationColumn = 3;
 const char* RipCDDialog::kSettingsGroup = "Transcoder";
 const int RipCDDialog::kMaxDestinationItems = 10;
 
-RipCDDialog::RipCDDialog(DeviceManager& device_manager, QWidget* parent)
+RipCDDialog::RipCDDialog(DeviceManager* device_manager, QWidget* parent)
     : QDialog(parent),
       ui_(new Ui_RipCDDialog),
       device_manager_(device_manager),
       cdda_devices_(
-          device_manager.FindDevicesByUrlSchemes(CddaDevice::url_schemes())),
+          device_manager->FindDevicesByUrlSchemes(CddaDevice::url_schemes())),
       working_(false),
       cdda_device_(),
       loader_(nullptr) {
+  Q_ASSERT(device_manager);
   // Init
   ui_->setupUi(this);
 
@@ -179,8 +180,10 @@ void RipCDDialog::ClickedRipButton() {
   Ripper* ripper = new Ripper(cdda_device_->raw_cdio(), this);
   connect(cancel_button_, SIGNAL(clicked()), ripper, SLOT(Cancel()));
 
-  connect(ripper, SIGNAL(Finished()), SLOT(Finished()));
-  connect(ripper, SIGNAL(Cancelled()), SLOT(Cancelled()));
+  connect(ripper, &Ripper::Finished, this,
+          [this, ripper]() { this->Finished(ripper); });
+  connect(ripper, &Ripper::Cancelled, this,
+          [this, ripper]() { this->Cancelled(ripper); });
   connect(ripper, SIGNAL(ProgressInterval(int, int)),
           SLOT(SetupProgressBarLimits(int, int)));
   connect(ripper, SIGNAL(Progress(int)), SLOT(UpdateProgressBar(int)));
@@ -279,7 +282,7 @@ void RipCDDialog::DeviceSelected(int device_index) {
 
   DeviceInfo* device_info = cdda_devices_[device_index];
   std::shared_ptr<ConnectedDevice> device =
-      device_manager_.Connect(device_info);
+      device_manager_->Connect(device_info);
   cdda_device_ = std::dynamic_pointer_cast<CddaDevice>(device);
   if (!cdda_device_) {
     rip_button_->setEnabled(false);
@@ -307,16 +310,14 @@ void RipCDDialog::DeviceSelected(int device_index) {
   rip_button_->setEnabled(true);
 }
 
-void RipCDDialog::Finished() {
+void RipCDDialog::Finished(Ripper* ripper) {
   SetWorking(false);
-  Ripper* ripper = dynamic_cast<Ripper*>(sender());
-  Q_ASSERT(ripper);
   ripper->deleteLater();
 }
 
-void RipCDDialog::Cancelled() {
+void RipCDDialog::Cancelled(Ripper* ripper) {
   ui_->progress_bar->setValue(0);
-  Finished();
+  Finished(ripper);
 }
 
 void RipCDDialog::SetupProgressBarLimits(int min, int max) {

src/ripper/ripcddialog.h

@@ -40,7 +40,7 @@ class RipCDDialog : public QDialog {
   Q_OBJECT
 
  public:
-  explicit RipCDDialog(DeviceManager& device_manager,
+  explicit RipCDDialog(DeviceManager* device_manager,
                        QWidget* parent = nullptr);
   ~RipCDDialog();
 
@@ -56,8 +56,8 @@ class RipCDDialog : public QDialog {
   void SelectNone();
   void InvertSelection();
   void DeviceSelected(int device_index);
-  void Finished();
-  void Cancelled();
+  void Finished(Ripper* ripper);
+  void Cancelled(Ripper* ripper);
   void SetupProgressBarLimits(int min, int max);
   void UpdateProgressBar(int progress);
   // Initializes track list table based on preliminary song list with durations
@@ -90,7 +90,7 @@ class RipCDDialog : public QDialog {
   QPushButton* close_button_;
   QPushButton* rip_button_;
   std::unique_ptr<Ui_RipCDDialog> ui_;
-  DeviceManager& device_manager_;
+  DeviceManager* device_manager_;
   QList<DeviceInfo*> cdda_devices_;
   bool working_;
   std::shared_ptr<CddaDevice> cdda_device_;

src/ui/mainwindow.cpp

@@ -2221,7 +2221,7 @@ void MainWindow::OpenRipCDDialog() {
 #ifdef HAVE_AUDIOCD
   if (!rip_cd_dialog_) {
     Q_ASSERT(app_->device_manager());
-    rip_cd_dialog_.reset(new RipCDDialog(*(app_->device_manager())));
+    rip_cd_dialog_.reset(new RipCDDialog(app_->device_manager()));
   }
   rip_cd_dialog_->show();
 #endif