GetDunne Wiki

Notes from the desk of Shane Dunne, software development consultant

User Tools

Site Tools

Trace: • avoid_new_delete_prefer_references_to_pointers_use_member_variables_for_sub-components
avoid_new_delete_prefer_references_to_pointers_use_member_variables_for_sub-components

Avoid new/delete, prefer references to pointers, use member variables for sub-Components

The JUCE library includes a nice collection of managed pointer template classes, such as juce::ScopedPointer, which can and should be used to avoid most uses of the delete keyword. Note that managed pointers are still assigned using new.

I checked my code carefully, and was unable to find any uses of delete, and found none. I did find a number of uses of new, however, which fell into two categories:

  1. In my VanillaJuceAudioProcessor class, I used new to create object-instances where there was effectively no choice; the JUCE library itself required it. I also used new when when creating juce::XmlElement objects, following the example of the JUCE static function juce::AudioProcessor::getXmlFromBinary(), which returns a non-managed pointer.
  2. In all of my GUI-related classes, I used new to create various juce::Component objects, assigning the result to juce::ScopedPointer member variables of the Gui… class. I used the Projucer to generate this code, and that's what it produced, so I assumed this was the recommended approach. I have since learned, through discussion on the JUCE Forum, that this is definitely not the case.

Regarding the first category, I can only assume that Jules's admonition to “ditch all your new/delete…” was something of a generic complaint. Regarding the GUI-related classes, I now understand that the Projucer's lavish use of ScopedPointer is not at all recommended, and that child Component objects should ideally be declared as member variables.

Cleaning up the GUI classes

JUCE Forum user Holy_City provided the following code-example showing a variety of tricks for reducing the size of constructor functions for juce::Component-derived classes: 

lass EnvelopeComponent : public Component
{
public:
    EnvelopeComponent() /* user the initializer list */
    :    attack (Slider::Rotary, Slider::noTextBox), //very helpful constructor for Sliders
         decay  (Slider::Rotary, Slider::noTextBox),
         sustain (Slider::Rotary, Slider::noTextBox),
         release (Slider::Rotary, Slider::noTextBox)
    {
        auto initSlider = [this] (Slider& s)  // lambda expression declaration
        {
            addAndMakeVisible (s):
            s.setRange (1.0, 500.0);
            s.setValue (25.0);
        };
 
         initSlider (attack);
         initSlider (decay);
         initSlider (release);
 
         addAndMakeVisible (sustain);
         sustain.setRange (0.0, 1.0);
         sustain.setValue (0.707);
    }
    //.... yada yada yada 
private:
    Slider attack, sustain, decay, release; /* don't need to put them on different lines */
};

Following Holy_City's example I was able to reduce the GuiEgTab class from this: 

class GuiEgTab  : public Component,
                  public SliderListener
{
public:
    GuiEgTab (SynthSound* pSynthSound);
    ~GuiEgTab();
 
    void paint (Graphics& g) override;
    void resized() override;
    void sliderValueChanged (Slider* sliderThatWasMoved) override;
 
    void notify();
 
private:
    SynthSound* pSound;
 
    ScopedPointer<Label> attackLabel;
    ScopedPointer<Slider> attackSlider;
    ScopedPointer<Label> decayLabel;
    ScopedPointer<Slider> decaySlider;
    ScopedPointer<Label> sustainLabel;
    ScopedPointer<Slider> sustainSlider;
    ScopedPointer<Label> releaseLabel;
    ScopedPointer<Slider> releaseSlider;
 
    JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (GuiEgTab)
};
 
GuiEgTab::GuiEgTab (SynthSound* pSynthSound)
    : pSound(pSynthSound)
{
    addAndMakeVisible (attackLabel = new Label ("attack label",
                                                TRANS("Attack Time (sec)")));
    attackLabel->setFont (Font (15.00f, Font::plain).withTypefaceStyle ("Regular"));
    attackLabel->setJustificationType (Justification::centredRight);
    attackLabel->setEditable (false, false, false);
    attackLabel->setColour (TextEditor::textColourId, Colours::black);
    attackLabel->setColour (TextEditor::backgroundColourId, Colour (0x00000000));
 
    addAndMakeVisible (attackSlider = new Slider ("attack time slider"));
    attackSlider->setRange (0, 10, 0);
    attackSlider->setSliderStyle (Slider::LinearHorizontal);
    attackSlider->setTextBoxStyle (Slider::TextBoxRight, false, 80, 20);
    attackSlider->addListener (this);
 
    addAndMakeVisible (decayLabel = new Label ("decay time label",
                                               TRANS("Decay Time (sec)")));
    decayLabel->setFont (Font (15.00f, Font::plain).withTypefaceStyle ("Regular"));
    decayLabel->setJustificationType (Justification::centredRight);
    decayLabel->setEditable (false, false, false);
    decayLabel->setColour (TextEditor::textColourId, Colours::black);
    decayLabel->setColour (TextEditor::backgroundColourId, Colour (0x00000000));
 
    addAndMakeVisible (decaySlider = new Slider ("decay time slider"));
    decaySlider->setRange (0, 10, 0);
    decaySlider->setSliderStyle (Slider::LinearHorizontal);
    decaySlider->setTextBoxStyle (Slider::TextBoxRight, false, 80, 20);
    decaySlider->addListener (this);
 
    addAndMakeVisible (sustainLabel = new Label ("sustain level label",
                                                 TRANS("Sustain Level (%)")));
    sustainLabel->setFont (Font (15.00f, Font::plain).withTypefaceStyle ("Regular"));
    sustainLabel->setJustificationType (Justification::centredRight);
    sustainLabel->setEditable (false, false, false);
    sustainLabel->setColour (TextEditor::textColourId, Colours::black);
    sustainLabel->setColour (TextEditor::backgroundColourId, Colour (0x00000000));
 
    addAndMakeVisible (sustainSlider = new Slider ("sustain level slider"));
    sustainSlider->setRange (0, 100, 1);
    sustainSlider->setSliderStyle (Slider::LinearHorizontal);
    sustainSlider->setTextBoxStyle (Slider::TextBoxRight, false, 80, 20);
    sustainSlider->addListener (this);
 
    addAndMakeVisible (releaseLabel = new Label ("release time label",
                                              TRANS("Release Time (sec)")));
    releaseLabel->setFont (Font (15.00f, Font::plain).withTypefaceStyle ("Regular"));
    releaseLabel->setJustificationType (Justification::centredRight);
    releaseLabel->setEditable (false, false, false);
    releaseLabel->setColour (TextEditor::textColourId, Colours::black);
    releaseLabel->setColour (TextEditor::backgroundColourId, Colour (0x00000000));
 
    addAndMakeVisible (releaseSlider = new Slider ("release time slider"));
    releaseSlider->setRange (0, 10, 0);
    releaseSlider->setSliderStyle (Slider::LinearHorizontal);
    releaseSlider->setTextBoxStyle (Slider::TextBoxRight, false, 80, 20);
    releaseSlider->addListener (this);
 
    notify();
}
 
GuiEgTab::~GuiEgTab()
{
}
 
//==============================================================================
void GuiEgTab::paint (Graphics& g)
{
    g.fillAll (Colour (0xff323e44));
}
 
void GuiEgTab::resized()
{
    const int labelLeft = 16;
    const int controlLeft = 144;
    const int labelWidth = 120;
    const int sliderWidth = 420;
    const int controlHeight = 24;
    const int gapHeight = 8;
 
    int top = 20;
    attackLabel->setBounds (labelLeft, top, labelWidth, controlHeight);
    attackSlider->setBounds (controlLeft, top, sliderWidth, controlHeight);
    top += controlHeight + gapHeight;
    decayLabel->setBounds (labelLeft, top, labelWidth, controlHeight);
    decaySlider->setBounds (controlLeft, top, sliderWidth, controlHeight);
    top += controlHeight + gapHeight;
    sustainLabel->setBounds (labelLeft, top, labelWidth, controlHeight);
    sustainSlider->setBounds (controlLeft, top, sliderWidth, controlHeight);
    top += controlHeight + gapHeight;
    releaseLabel->setBounds (labelLeft, top, labelWidth, controlHeight);
    releaseSlider->setBounds (controlLeft, top, sliderWidth, controlHeight);
}
 
void GuiEgTab::sliderValueChanged (Slider* sliderThatWasMoved)
{
    double value = sliderThatWasMoved->getValue();
    SynthParameters* pParams = pSound->pParams;
    if (sliderThatWasMoved == attackSlider)
    {
        pParams->ampEgAttackTimeSeconds = value;
    }
    else if (sliderThatWasMoved == decaySlider)
    {
        pParams->ampEgDecayTimeSeconds = value;
    }
    else if (sliderThatWasMoved == sustainSlider)
    {
        pParams->ampEgSustainLevel = 0.01 * value;
    }
    else if (sliderThatWasMoved == releaseSlider)
    {
        pParams->ampEgReleaseTimeSeconds = value;
    }
    pSound->parameterChanged();
}
 
void GuiEgTab::notify()
{
    SynthParameters* pParams = pSound->pParams;
    attackSlider->setValue(pParams->ampEgAttackTimeSeconds);
    decaySlider->setValue(pParams->ampEgDecayTimeSeconds);
    sustainSlider->setValue(100.0 * pParams->ampEgSustainLevel);
    releaseSlider->setValue(pParams->ampEgReleaseTimeSeconds);
}

to this: 

class GuiEgTab  : public Component,
                  public SliderListener
{
public:
    GuiEgTab (SynthSound* pSynthSound);
 
    void paint (Graphics& g) override;
    void resized() override;
    void sliderValueChanged (Slider* sliderThatWasMoved) override;
 
    void notify();
 
private:
    SynthSound* pSound;
 
    Label attackLabel, decayLabel, sustainLabel, releaseLabel;
    Slider attackSlider, decaySlider, sustainSlider, releaseSlider;
 
    JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (GuiEgTab)
};
 
GuiEgTab::GuiEgTab (SynthSound* pSynthSound)
    : pSound(pSynthSound)
    , attackLabel("attack", TRANS("Attack Time (sec)"))
    , decayLabel("decay", TRANS("Decay Time (sec)"))
    , sustainLabel("sustain", TRANS("Sustain Level (%)"))
    , releaseLabel("release", TRANS("Release Time (sec)"))
{
    auto initLabel = [this](Label& label)
    {
        addAndMakeVisible(label);
        label.setFont (Font (15.00f, Font::plain).withTypefaceStyle ("Regular"));
        label.setJustificationType (Justification::centredRight);
        label.setEditable (false, false, false);
        label.setColour (TextEditor::textColourId, Colours::black);
        label.setColour (TextEditor::backgroundColourId, Colour (0x00000000));
    };
 
    initLabel(attackLabel);
    initLabel(decayLabel);
    initLabel(sustainLabel);
    initLabel(releaseLabel);
 
    auto initSlider = [this](Slider& slider)
    {
        addAndMakeVisible(slider);
        slider.setSliderStyle (Slider::LinearHorizontal);
        slider.setTextBoxStyle (Slider::TextBoxRight, false, 80, 20);
        slider.addListener (this);
    };
 
    initSlider(attackSlider); attackSlider.setRange (0, 10, 0);
    initSlider(decaySlider); decaySlider.setRange (0, 10, 0);
    initSlider(sustainSlider); sustainSlider.setRange (0, 100, 1);
    initSlider(releaseSlider); releaseSlider.setRange (0, 10, 0);
 
    notify();
}
 
void GuiEgTab::paint (Graphics& g)
{
    g.fillAll (Colour (0xff323e44));
}
 
void GuiEgTab::resized()
{
    const int labelLeft = 16;
    const int controlLeft = 144;
    const int labelWidth = 120;
    const int sliderWidth = 420;
    const int controlHeight = 24;
    const int gapHeight = 8;
 
    int top = 20;
    attackLabel.setBounds (labelLeft, top, labelWidth, controlHeight);
    attackSlider.setBounds (controlLeft, top, sliderWidth, controlHeight);
    top += controlHeight + gapHeight;
    decayLabel.setBounds (labelLeft, top, labelWidth, controlHeight);
    decaySlider.setBounds (controlLeft, top, sliderWidth, controlHeight);
    top += controlHeight + gapHeight;
    sustainLabel.setBounds (labelLeft, top, labelWidth, controlHeight);
    sustainSlider.setBounds (controlLeft, top, sliderWidth, controlHeight);
    top += controlHeight + gapHeight;
    releaseLabel.setBounds (labelLeft, top, labelWidth, controlHeight);
    releaseSlider.setBounds (controlLeft, top, sliderWidth, controlHeight);
}
 
void GuiEgTab::sliderValueChanged (Slider* sliderThatWasMoved)
{
    double value = sliderThatWasMoved->getValue();
    SynthParameters* pParams = pSound->pParams;
    if (sliderThatWasMoved == &attackSlider) pParams->ampEgAttackTimeSeconds = value;
    else if (sliderThatWasMoved == &decaySlider) pParams->ampEgDecayTimeSeconds = value;
    else if (sliderThatWasMoved == &sustainSlider) pParams->ampEgSustainLevel = 0.01 * value;
    else if (sliderThatWasMoved == &releaseSlider) pParams->ampEgReleaseTimeSeconds = value;
    pSound->parameterChanged();
}
 
void GuiEgTab::notify()
{
    SynthParameters* pParams = pSound->pParams;
    attackSlider.setValue(pParams->ampEgAttackTimeSeconds);
    decaySlider.setValue(pParams->ampEgDecayTimeSeconds);
    sustainSlider.setValue(100.0 * pParams->ampEgSustainLevel);
    releaseSlider.setValue(pParams->ampEgReleaseTimeSeconds);
}
avoid_new_delete_prefer_references_to_pointers_use_member_variables_for_sub-components.txt · Last modified: 2017/09/01 19:02 by shane

Page Tools

Except where otherwise noted, content on this wiki is licensed under the following license: CC Attribution 4.0 International
CC Attribution 4.0 International Donate Powered by PHP Valid HTML5 Valid CSS Driven by DokuWiki