From eb1e67583f6526f7362cda672eb593c7671366da Mon Sep 17 00:00:00 2001 From: John Beard Date: Wed, 15 May 2019 18:07:34 +0100 Subject: [PATCH] Pcbnew: tidy up array options access/allocation Use unique_ptrs for ownership transfer. Pass the target object to the constructor rather than creating an internal verison. --- pcbnew/array_creator.cpp | 7 +-- pcbnew/dialogs/dialog_create_array.cpp | 45 +++++++--------- pcbnew/dialogs/dialog_create_array.h | 71 ++++++++++++-------------- 3 files changed, 54 insertions(+), 69 deletions(-) diff --git a/pcbnew/array_creator.cpp b/pcbnew/array_creator.cpp index 953532feaf..c7cfb27be4 100644 --- a/pcbnew/array_creator.cpp +++ b/pcbnew/array_creator.cpp @@ -64,10 +64,11 @@ void ARRAY_CREATOR::Invoke() const bool enableArrayNumbering = isModuleEditor; const wxPoint rotPoint = getRotationCentre(); - DIALOG_CREATE_ARRAY dialog( &m_parent, enableArrayNumbering, rotPoint ); - int ret = dialog.ShowModal(); + std::unique_ptr array_opts; - ARRAY_OPTIONS* const array_opts = dialog.GetArrayOptions(); + DIALOG_CREATE_ARRAY dialog( &m_parent, array_opts, enableArrayNumbering, rotPoint ); + + int ret = dialog.ShowModal(); if( ret != wxID_OK || array_opts == NULL ) return; diff --git a/pcbnew/dialogs/dialog_create_array.cpp b/pcbnew/dialogs/dialog_create_array.cpp index 701fa644fc..cc4951d392 100644 --- a/pcbnew/dialogs/dialog_create_array.cpp +++ b/pcbnew/dialogs/dialog_create_array.cpp @@ -130,10 +130,12 @@ static const std::vector numberingTypeData { }, }; -DIALOG_CREATE_ARRAY::DIALOG_CREATE_ARRAY( - PCB_BASE_FRAME* aParent, bool enableNumbering, wxPoint aOrigPos ) +DIALOG_CREATE_ARRAY::DIALOG_CREATE_ARRAY( PCB_BASE_FRAME* aParent, + std::unique_ptr& aSettings, bool enableNumbering, wxPoint aOrigPos ) : DIALOG_CREATE_ARRAY_BASE( aParent ), - m_settings( NULL ), + m_settings( aSettings ), + m_originalItemPosition( aOrigPos ), + m_numberingEnabled( enableNumbering ), m_hSpacing( aParent, m_labelDx, m_entryDx, m_unitLabelDx ), m_vSpacing( aParent, m_labelDy, m_entryDy, m_unitLabelDy ), m_hOffset( aParent, m_labelOffsetX, m_entryOffsetX, m_unitLabelOffsetX ), @@ -142,9 +144,7 @@ DIALOG_CREATE_ARRAY::DIALOG_CREATE_ARRAY( m_vCentre( aParent, m_labelCentreY, m_entryCentreY, m_unitLabelCentreY ), m_circRadius( aParent, m_labelCircRadius, m_valueCircRadius, m_unitLabelCircRadius ), m_circAngle( aParent, m_labelCircAngle, m_entryCircAngle, m_unitLabelCircAngle ), - m_cfg_persister( saved_array_options.m_optionsSet ), - m_originalItemPosition( aOrigPos ), - m_numberingEnabled( enableNumbering ) + m_cfg_persister( saved_array_options.m_optionsSet ) { // Set up numbering scheme drop downs character set strings for( const auto& numData : numberingTypeData ) @@ -215,13 +215,6 @@ DIALOG_CREATE_ARRAY::DIALOG_CREATE_ARRAY( } -DIALOG_CREATE_ARRAY::~DIALOG_CREATE_ARRAY() -{ - if( m_settings != NULL ) - delete m_settings; -} - - void DIALOG_CREATE_ARRAY::OnParameterChanged( wxCommandEvent& event ) { setControlEnablement(); @@ -297,13 +290,14 @@ static bool validateLongEntry( const wxTextEntry& entry, long& dest, const wxStr bool DIALOG_CREATE_ARRAY::TransferDataFromWindow() { - ARRAY_OPTIONS* newSettings = NULL; + std::unique_ptr newSettings; + wxArrayString errors; const wxWindow* page = m_gridTypeNotebook->GetCurrentPage(); if( page == m_gridPanel ) { - ARRAY_GRID_OPTIONS* newGrid = new ARRAY_GRID_OPTIONS(); + auto newGrid = std::make_unique(); bool ok = true; // ints @@ -357,13 +351,11 @@ bool DIALOG_CREATE_ARRAY::TransferDataFromWindow() // Only use settings if all values are good if( ok ) - newSettings = newGrid; - else - delete newGrid; + newSettings = std::move( newGrid ); } else if( page == m_circularPanel ) { - ARRAY_CIRCULAR_OPTIONS* newCirc = new ARRAY_CIRCULAR_OPTIONS(); + auto newCirc = std::make_unique(); bool ok = true; newCirc->m_centre.x = m_hCentre.GetValue(); @@ -381,8 +373,9 @@ bool DIALOG_CREATE_ARRAY::TransferDataFromWindow() if( newCirc->GetNumberingStartIsSpecified() ) { - ok = ok && validateNumberingTypeAndOffset( *m_entryCircNumberingStart, - *m_choiceCircNumbering, newCirc->m_axis, errors ); + ok = ok + && validateNumberingTypeAndOffset( *m_entryCircNumberingStart, + *m_choiceCircNumbering, newCirc->m_axis, errors ); } else { @@ -394,18 +387,16 @@ bool DIALOG_CREATE_ARRAY::TransferDataFromWindow() // Only use settings if all values are good if( ok ) - newSettings = newCirc; - else - delete newCirc; + newSettings = std::move( newCirc ); } // If we got good settings, send them out and finish if( newSettings ) { - delete m_settings; - // assign pointer and ownership here - m_settings = newSettings; + m_settings = std::move( newSettings ); + + // persist the control state for next time m_cfg_persister.ReadConfigFromControls(); return true; diff --git a/pcbnew/dialogs/dialog_create_array.h b/pcbnew/dialogs/dialog_create_array.h index 8055ac2c75..fb1b9a2315 100644 --- a/pcbnew/dialogs/dialog_create_array.h +++ b/pcbnew/dialogs/dialog_create_array.h @@ -22,8 +22,8 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA */ -#ifndef __DIALOG_CREATE_ARRAY__ -#define __DIALOG_CREATE_ARRAY__ +#ifndef DIALOG_CREATE_ARRAY__H_ +#define DIALOG_CREATE_ARRAY__H_ // Include the wxFormBuider header base: #include @@ -32,53 +32,28 @@ #include #include -#include #include #include +#include class DIALOG_CREATE_ARRAY : public DIALOG_CREATE_ARRAY_BASE { public: - - // Constructor and destructor - DIALOG_CREATE_ARRAY( PCB_BASE_FRAME* aParent, bool enableNumbering, - wxPoint aOrigPos ); - - ~DIALOG_CREATE_ARRAY(); - - /*! - * @return the array options set by this dialogue, or NULL if they were - * not set, or could not be set + /** + * Construct a new dialog. + * + * @param aParent the parent window + * @param aOptions the options that will be re-seated when dialog is validly closed + * @param aEnableNumbering enable pad numbering + * @param aOrigPos original item position (used for computing the circular array radius) */ - ARRAY_OPTIONS* GetArrayOptions() const - { - return m_settings; - } + DIALOG_CREATE_ARRAY( PCB_BASE_FRAME* aParent, std::unique_ptr& aOptions, + bool enableNumbering, wxPoint aOrigPos ); private: - - /** - * The settings object returned to the caller. - * We retain ownership of this - */ - ARRAY_OPTIONS* m_settings; - - UNIT_BINDER m_hSpacing, m_vSpacing; - UNIT_BINDER m_hOffset, m_vOffset; - UNIT_BINDER m_hCentre, m_vCentre; - UNIT_BINDER m_circRadius; - UNIT_BINDER m_circAngle; - - WIDGET_SAVE_RESTORE m_cfg_persister; - - /* - * The position of the original item(s), used for finding radius, etc - */ - const wxPoint m_originalItemPosition; - // Event callbacks - void OnParameterChanged( wxCommandEvent& event ) override; + void OnParameterChanged( wxCommandEvent& event ) override; // Internal callback handlers void setControlEnablement(); @@ -86,8 +61,26 @@ private: bool TransferDataFromWindow() override; + /** + * The settings to re-seat on dialog OK. + */ + std::unique_ptr& m_settings; + + /* + * The position of the original item(s), used for finding radius, etc + */ + const wxPoint m_originalItemPosition; + // some uses of arrays might not allow component renumbering bool m_numberingEnabled; + + UNIT_BINDER m_hSpacing, m_vSpacing; + UNIT_BINDER m_hOffset, m_vOffset; + UNIT_BINDER m_hCentre, m_vCentre; + UNIT_BINDER m_circRadius; + UNIT_BINDER m_circAngle; + + WIDGET_SAVE_RESTORE m_cfg_persister; }; -#endif // __DIALOG_CREATE_ARRAY__ +#endif // DIALOG_CREATE_ARRAY__H_