GetDunne Wiki

Notes from the desk of Shane Dunne, software development consultant

User Tools

Site Tools


avoid_new_delete_prefer_references_to_pointers_use_member_variables_for_sub-components

Differences

This shows you the differences between two versions of the page.

Link to this comparison view

Both sides previous revision Previous revision
avoid_new_delete_prefer_references_to_pointers_use_member_variables_for_sub-components [2017/09/01 18:22]
shane
avoid_new_delete_prefer_references_to_pointers_use_member_variables_for_sub-components [2017/09/01 19:02] (current)
shane
Line 6: Line 6:
   - 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.   - 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 be +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: 
 +<code cpp> 
 +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 */ 
 +}; 
 +</code> 
 + 
 +Following //Holy_City//'s example I was able to reduce the //GuiEgTab// class from this: 
 +<code cpp> 
 +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); 
 +
 +</code> 
 +to this: 
 +<code cpp> 
 +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); 
 +
 +</code> 
avoid_new_delete_prefer_references_to_pointers_use_member_variables_for_sub-components.1504290177.txt.gz · Last modified: 2017/09/01 18:22 by shane