GetDunne Wiki

Notes from the desk of Shane Dunne, software development consultant

User Tools

Site Tools

Trace: • sarah
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

Next revision 		Previous revision
avoid_new_delete_prefer_references_to_pointers_use_member_variables_for_sub-components [2017/09/01 18:06]
shane created 		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 ideally be declared as member variables.
  
-  - In my //VanillaJuceAudioProcessor// class, I use ''new'' to create instances of //SynthVoice// and //SynthSound//, which are immediately added to the //Synth// object using //juce::Synthesiser::addVoice()// and //juce::Synthesiser::addSound()//, for both of which the JUCE documentation states that the //juce::Synthesiser// object will take care of deleting the passed-in object when required. As far as I could tell, there is no other way to add voices/sounds to a //juce::Synthesiser// other than by passing in a pointer, so these uses of ''new'' are unavoidable. +===== Cleaning up the GUI classes ===== 
-  - Also in my //VanillaJuceAudioProcessor// class, in places where I used JUCE's built-in +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>
  
-so I have to assume that Jules's admonition to "ditch all your new/delete..." was something of a generic complaint. 
avoid_new_delete_prefer_references_to_pointers_use_member_variables_for_sub-components.1504289213.txt.gz · Last modified: 2017/09/01 18:06 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