Apply C++ best practices

Author: timothyschoen

Source/Canvas.cpp

@@ -92,9 +92,9 @@ class ObjectsResizer final : public Component
         addAndMakeVisible(border);
 
         setBounds(selectedObjectBounds.expanded(tabMargin));
-    };
+    }
 
-    ~ObjectsResizer()
+    ~ObjectsResizer() override
     {
         for (auto* obj : cnv->getSelectionOfType<Object>()) {
             obj->hideHandles(false);
@@ -567,6 +567,7 @@ void Canvas::updateFramebuffers(NVGcontext* nvg)
                 if (zoom < 0.5f)
                     decim = 12;
                 break;
+            default: break;
             }
 
             auto const majorDotColour = canvasMarkingsColJuce.withAlpha(std::min(zoom * 0.8f, 1.0f));
@@ -892,6 +893,7 @@ void Canvas::settingsChanged(String const& name, var const& value)
         updateOverlays();
         break;
     }
+    default: break;
     }
 }
 
@@ -1111,7 +1113,7 @@ void Canvas::synchroniseSplitCanvas()
 void Canvas::performSynchronise()
 {
     static bool alreadyFlushed = false;
-    bool needsFlush = !alreadyFlushed;
+    bool const needsFlush = !alreadyFlushed;
     ScopedValueSetter<bool> flushGuard(alreadyFlushed, true);
     // By flushing twice, we can make sure that any message sent before this point will be dequeued
     if (needsFlush && !isGraph)
@@ -1126,10 +1128,8 @@ void Canvas::performSynchronise()
 
     // Remove deleted objects
     for (int n = objects.size() - 1; n >= 0; n--) {
-        auto* object = objects[n];
-
         // If the object is showing it's initial editor, meaning no object was assigned yet, allow it to exist without pointing to an object
-        if (!object->getPointer() && !object->isInitialEditorShown()) {
+        if (auto* object = objects[n]; !object->getPointer() && !object->isInitialEditorShown()) {
             setSelected(object, false, false);
             objects.remove_at(n);
         }
@@ -1173,7 +1173,7 @@ void Canvas::performSynchronise()
 
     // Make sure objects have the same order
     std::ranges::sort(objects,
-        [&pdObjects](Object* first, Object* second) mutable {
+        [&pdObjects](Object const* first, Object const* second) mutable {
             return pdObjects.index_of(first->getPointer()) < pdObjects.index_of(second->getPointer());
         });
 
@@ -1221,11 +1221,9 @@ void Canvas::performSynchronise()
         if (it == connections.end()) {
             connections.add(this, inlet, outlet, ptr);
         } else {
-            auto& c = **it;
-
             // This is necessary to make resorting a subpatchers iolets work
             // And it can't hurt to check if the connection is valid anyway
-            if (c.inlet != inlet || c.outlet != outlet) {
+            if (auto& c = **it; c.inlet != inlet || c.outlet != outlet) {
                 int const idx = connections.index_of(*it);
                 connections.remove_one(*it);
                 connections.insert(idx, this, inlet, outlet, ptr);
@@ -1306,8 +1304,7 @@ void Canvas::shiftKeyChanged(bool const isHeld)
                     return;
 
                 t_outconnect* connection = nullptr;
-                auto selectedConnections = getSelectionOfType<Connection>();
-                if (selectedConnections.size() == 1) {
+                if (auto selectedConnections = getSelectionOfType<Connection>(); selectedConnections.size() == 1) {
                     connection = selectedConnections[0]->getPointer();
                 }
 
@@ -1428,10 +1425,8 @@ void Canvas::mouseDrag(MouseEvent const& e)
         }
     }
 
-    bool const objectIsBeingEdited = ObjectBase::isBeingEdited();
-
     // Ignore on graphs or when locked
-    if ((isGraph || locked == var(true) || commandLocked == var(true)) && !objectIsBeingEdited) {
+    if ((isGraph || locked == var(true) || commandLocked == var(true)) && !ObjectBase::isBeingEdited()) {
         bool hasToggled = false;
 
         // Behaviour for dragging over toggles, bang and radiogroup to toggle them
@@ -1733,7 +1728,7 @@ void Canvas::focusLost(FocusChangeType cause)
     });
 }
 
-void Canvas::dragAndDropPaste(String const& patchString, Point<int> mousePos, int const patchWidth, int const patchHeight, String const& name)
+void Canvas::dragAndDropPaste(String const& patchString, Point<int> const mousePos, int const patchWidth, int const patchHeight, String const& name)
 {
     locked = false;
     presentationMode = false;
@@ -2017,9 +2012,7 @@ void Canvas::cycleSelection()
         return;
     }
     // Get the selected objects
-    auto selectedObjects = getSelectionOfType<Object>();
-
-    if (selectedObjects.size() == 1) {
+    if (auto selectedObjects = getSelectionOfType<Object>(); selectedObjects.size() == 1) {
         // Find the index of the currently selected object
         auto const currentIdx = objects.index_of(selectedObjects[0]);
         setSelected(selectedObjects[0], false);
@@ -2032,9 +2025,7 @@ void Canvas::cycleSelection()
     }
 
     // Get the selected connections if no objects are selected
-    auto selectedConnections = getSelectionOfType<Connection>();
-
-    if (selectedConnections.size() == 1) {
+    if (auto selectedConnections = getSelectionOfType<Connection>(); selectedConnections.size() == 1) {
         // Find the index of the currently selected connection
         auto const currentIdx = connections.index_of(selectedConnections[0]);
         setSelected(selectedConnections[0], false);
@@ -2237,8 +2228,7 @@ void Canvas::connectSelection()
     }
 
     t_outconnect* connection = nullptr;
-    auto selectedConnections = getSelectionOfType<Connection>();
-    if (selectedConnections.size() == 1) {
+    if (auto selectedConnections = getSelectionOfType<Connection>(); selectedConnections.size() == 1) {
         connection = selectedConnections[0]->getPointer();
     }
 
@@ -2316,7 +2306,7 @@ void Canvas::alignObjects(Align const alignment)
     // get the bounding box of all selected objects
     auto const selectedBounds = getBoundingBox(selectedObjects);
 
-    auto onMove = [this, selectedObjects](Point<int> position) {
+    auto onMove = [this, selectedObjects](Point<int> const position) {
         // Calculate the bounding box of all selected objects
         Rectangle<int> totalSize;
 
@@ -2389,7 +2379,7 @@ void Canvas::alignObjects(Align const alignment)
     case Align::HDistribute: {
         sortByXPos(selectedObjects);
 
-        auto onResize = [this, selectedObjects](Rectangle<int> newBounds) {
+        auto onResize = [this, selectedObjects](Rectangle<int> const newBounds) {
             int totalObjectsWidth = 0;
             for (auto const* obj : selectedObjects) {
                 totalObjectsWidth += obj->getBounds().getWidth() - Object::doubleMargin;
@@ -2419,7 +2409,7 @@ void Canvas::alignObjects(Align const alignment)
     case Align::VDistribute: {
         sortByYPos(selectedObjects);
 
-        auto onResize = [this, selectedObjects](Rectangle<int> newBounds) {
+        auto onResize = [this, selectedObjects](Rectangle<int> const newBounds) {
             int totalObjectsHeight = 0;
             for (auto const* obj : selectedObjects) {
                 totalObjectsHeight += obj->getBounds().getHeight() - Object::doubleMargin;
@@ -2516,7 +2506,7 @@ void Canvas::valueChanged(Value& v)
             auto y2 = static_cast<float>(cnv->gl_screeny2);
 
             char buf[MAXPDSTRING];
-            snprintf(buf, MAXPDSTRING - 1, ".x%lx", (unsigned long)cnv.get());
+            snprintf(buf, MAXPDSTRING - 1, ".x%lx", reinterpret_cast<unsigned long>(cnv.get()));
             pd->sendMessage(buf, "setbounds", { x1, y1, x2, y2 });
         }
         repaint();
@@ -2529,7 +2519,7 @@ void Canvas::valueChanged(Value& v)
             auto y2 = static_cast<float>(getValue<int>(patchHeight) + y1);
 
             char buf[MAXPDSTRING];
-            snprintf(buf, MAXPDSTRING - 1, ".x%lx", (unsigned long)cnv.get());
+            snprintf(buf, MAXPDSTRING - 1, ".x%lx", reinterpret_cast<unsigned long>(cnv.get()));
             pd->sendMessage(buf, "setbounds", { x1, y1, x2, y2 });
         }
         repaint();
@@ -2676,7 +2666,7 @@ bool Canvas::setPanDragMode(bool const shouldPan)
     return false;
 }
 
-bool Canvas::isPointOutsidePluginArea(Point<int> point) const
+bool Canvas::isPointOutsidePluginArea(Point<int> const point) const
 {
     auto const borderWidth = getValue<float>(patchWidth);
     auto const borderHeight = getValue<float>(patchHeight);
@@ -2786,8 +2776,7 @@ void Canvas::receiveMessage(t_symbol* symbol, SmallArray<pd::Atom> const& atoms)
             return;
 
         if (atoms.size() >= 1) {
-            int const flag = atoms[0].getFloat();
-            if (flag % 2 == 0) {
+            if (int const flag = atoms[0].getFloat(); flag % 2 == 0) {
                 locked = true;
             } else {
                 locked = false;
@@ -2812,6 +2801,7 @@ void Canvas::receiveMessage(t_symbol* symbol, SmallArray<pd::Atom> const& atoms)
         synchroniseSplitCanvas();
         break;
     }
+    default: break;
     }
 }
 

Source/CanvasViewport.h

@@ -22,27 +22,27 @@ using namespace gl;
 
 #include "Utility/SettingsFile.h"
 
-class Minimap : public Component
+class Minimap final : public Component
     , public Timer
     , public AsyncUpdater {
 public:
-    Minimap(Canvas* canvas)
+    explicit Minimap(Canvas* canvas)
         : cnv(canvas)
     {
     }
 
     void handleAsyncUpdate() override
     {
-        auto area = visibleArea / getValue<float>(cnv->zoomScale);
+        auto const area = visibleArea / getValue<float>(cnv->zoomScale);
         bool renderMinimap = cnv->objects.not_empty();
-        for (auto* obj : cnv->objects) {
+        for (auto const* obj : cnv->objects) {
             if (obj->getBounds().intersects(area)) {
                 renderMinimap = false;
                 break;
             }
         }
 
-        auto showMinimap = SettingsFile::getInstance()->getProperty<int>("show_minimap");
+        auto const showMinimap = SettingsFile::getInstance()->getProperty<int>("show_minimap");
         float fadedIn = 0.0f;
         float fadedOut = 0.0f;
         if (showMinimap == 1) {
@@ -71,7 +71,7 @@ class Minimap : public Component
         }
     }
 
-    void updateMinimap(Rectangle<int> area)
+    void updateMinimap(Rectangle<int> const area)
     {
         if (isMouseDown || area.isEmpty())
             return;
@@ -91,8 +91,8 @@ class Minimap : public Component
 
         auto const zoom = getValue<float>(cnv->zoomScale);
         b.viewBounds = cnv->viewport->getViewArea() / zoom;
-        Rectangle<int> allObjectBounds = Rectangle<int>(cnv->canvasOrigin.x, cnv->canvasOrigin.y, b.viewBounds.getWidth(), b.viewBounds.getHeight());
-        for (auto* object : cnv->objects) {
+        auto allObjectBounds = Rectangle<int>(cnv->canvasOrigin.x, cnv->canvasOrigin.y, b.viewBounds.getWidth(), b.viewBounds.getHeight());
+        for (auto const* object : cnv->objects) {
             allObjectBounds = allObjectBounds.getUnion(object->getBounds());
         }
 
@@ -118,8 +118,8 @@ class Minimap : public Component
         float const x = cnv->viewport->getViewWidth() - (width + 10);
         float const y = cnv->viewport->getViewHeight() - (height + 10);
 
-        auto canvasBackground = cnv->findColour(PlugDataColour::canvasBackgroundColourId);
-        auto mapBackground = canvasBackground.contrasting(0.5f);
+        auto const canvasBackground = cnv->findColour(PlugDataColour::canvasBackgroundColourId);
+        auto const mapBackground = canvasBackground.contrasting(0.5f);
 
         // draw background
         nvgFillColor(nvg, NVGComponent::convertColour(mapBackground.withAlpha(0.4f)));
@@ -155,13 +155,13 @@ class Minimap : public Component
         downPosition = cnv->viewport->getViewPosition();
 
         auto map = getMapBounds();
-        auto realViewBounds = Rectangle<int>((map.offsetX + map.viewBounds.getX() - cnv->canvasOrigin.x) * map.scale, (map.offsetY + map.viewBounds.getY() - cnv->canvasOrigin.y) * map.scale, map.viewBounds.getWidth() * map.scale, map.viewBounds.getHeight() * map.scale);
+        auto const realViewBounds = Rectangle<int>((map.offsetX + map.viewBounds.getX() - cnv->canvasOrigin.x) * map.scale, (map.offsetY + map.viewBounds.getY() - cnv->canvasOrigin.y) * map.scale, map.viewBounds.getWidth() * map.scale, map.viewBounds.getHeight() * map.scale);
         isMouseDown = realViewBounds.contains(e.getMouseDownPosition());
     }
 
     void mouseUp(MouseEvent const& e) override
     {
-        auto viewBounds = cnv->viewport->getViewArea();
+        auto const viewBounds = cnv->viewport->getViewArea();
         downPosition = viewBounds.getPosition();
         isMouseDown = false;
         updateMinimap(viewBounds);
@@ -173,7 +173,7 @@ class Minimap : public Component
         auto map = getMapBounds();
 
         if (isMouseDown) {
-            cnv->viewport->setViewPosition(downPosition + (e.getOffsetFromDragStart() / map.scale));
+            cnv->viewport->setViewPosition(downPosition + e.getOffsetFromDragStart() / map.scale);
         }
     }
 
@@ -488,7 +488,7 @@ class CanvasViewport final : public Viewport
         addAndMakeVisible(vbar);
         addAndMakeVisible(hbar);
 
-        setCachedComponentImage(new NVGSurface::InvalidationListener(editor->nvgSurface, this, [this](){
+        setCachedComponentImage(new NVGSurface::InvalidationListener(editor->nvgSurface, this, [this]{
             return editor->getTabComponent().getVisibleCanvases().contains(this->cnv);
         }));
 
@@ -547,6 +547,7 @@ class CanvasViewport final : public Viewport
             lerpAnimation += animationSpeed;
             break;
         }
+        default: break;
         }
     }
 

Source/Components/Buttons.cpp

@@ -140,7 +140,7 @@ void ToolbarRadioButton::mouseExit(MouseEvent const& e)
 
 
 
-bool SmallIconButton::hitTest(int x, int y)
+bool SmallIconButton::hitTest(int const x, int const y)
 {
     if (getLocalBounds().reduced(2).contains(x, y))
         return true;
@@ -176,7 +176,7 @@ void SmallIconButton::paint(Graphics& g)
 
 WidePanelButton::WidePanelButton(String const& icon, int const iconSize)
 : icon(icon)
-, iconSize(iconSize) { };
+, iconSize(iconSize) { }
 
 void WidePanelButton::mouseEnter(MouseEvent const& e)
 {

Source/Components/Buttons.h

@@ -22,7 +22,7 @@ class MainToolbarButton final : public TextButton {
 
     // On macOS, we need to make sure that dragging any of these buttons doesn't drag the whole titlebar
 #if JUCE_MAC
-    ~MainToolbarButton();
+    ~MainToolbarButton() override;
     void mouseEnter(MouseEvent const& e) override;
     void mouseExit(MouseEvent const& e) override;
 #endif
@@ -57,7 +57,7 @@ class WidePanelButton final : public TextButton {
     int iconSize;
 
 public:
-    explicit WidePanelButton(String const& icon, int const iconSize = 13);
+    explicit WidePanelButton(String const& icon, int iconSize = 13);
 
     void mouseEnter(MouseEvent const& e) override;
 

Source/Components/ColourPicker.h

@@ -51,7 +51,7 @@ class Eyedropper final : public Timer
             return false;
         }
 
-        void setROI(Image& image, Point<int> position)
+        void setROI(Image const& image, Point<int> const position)
         {
             pixelImage = image.getClippedImage(Rectangle<int>(0, 0, 11, 11).withCentre(position)).rescaled(getWidth() - 8 * 2, getHeight() - 8 * 2, Graphics::ResamplingQuality::lowResamplingQuality);
             colour = image.getPixelAt(position.x, position.y);
@@ -212,7 +212,7 @@ class ColourPicker final : public Component {
     };
 
 public:
-    void show(PluginEditor* editorComponent, Component* topLevelComponent, bool const onlySendCallbackOnClose, Colour const currentColour, Rectangle<int> bounds, std::function<void(Colour)> const& colourCallback)
+    void show(PluginEditor* editorComponent, Component* topLevelComponent, bool const onlySendCallbackOnClose, Colour const currentColour, Rectangle<int> const bounds, std::function<void(Colour)> const& colourCallback)
     {
         callback = colourCallback;
         onlyCallBackOnClose = onlySendCallbackOnClose;

Source/Components/ConnectionMessageDisplay.h

@@ -45,7 +45,7 @@ class ConnectionMessageDisplay final
     }
 
     // Activate the current connection info display overlay, to hide give it a nullptr
-    void setConnection(Connection* connection, Point<int> screenPosition = { 0, 0 })
+    void setConnection(Connection* connection, Point<int> const screenPosition = { 0, 0 })
     {
         // multiple events can hide the display, so we don't need to do anything
         // if this object has already been set to null
@@ -123,7 +123,7 @@ class ConnectionMessageDisplay final
                         for(int ch = 0; ch < numChannels; ch++)
                         {
                             auto* start = samples + (ch * numSamples + block * blockSize);
-                            auto* destination = output.data() + (ch * blockSize);
+                            auto* destination = output.data() + ch * blockSize;
                             std::copy_n(start, blockSize, destination);
                         }
                         sampleQueue.try_enqueue(SignalBlock(std::move(output), numChannels, blockSize));