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.
This commit is contained in:
John Beard 2019-05-15 18:07:34 +01:00
parent 6ade8c25e1
commit eb1e67583f
3 changed files with 54 additions and 69 deletions

View File

@ -64,10 +64,11 @@ void ARRAY_CREATOR::Invoke()
const bool enableArrayNumbering = isModuleEditor; const bool enableArrayNumbering = isModuleEditor;
const wxPoint rotPoint = getRotationCentre(); const wxPoint rotPoint = getRotationCentre();
DIALOG_CREATE_ARRAY dialog( &m_parent, enableArrayNumbering, rotPoint ); std::unique_ptr<ARRAY_OPTIONS> array_opts;
int ret = dialog.ShowModal();
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 ) if( ret != wxID_OK || array_opts == NULL )
return; return;

View File

@ -130,10 +130,12 @@ static const std::vector<NUMBERING_LIST_DATA> numberingTypeData {
}, },
}; };
DIALOG_CREATE_ARRAY::DIALOG_CREATE_ARRAY( DIALOG_CREATE_ARRAY::DIALOG_CREATE_ARRAY( PCB_BASE_FRAME* aParent,
PCB_BASE_FRAME* aParent, bool enableNumbering, wxPoint aOrigPos ) std::unique_ptr<ARRAY_OPTIONS>& aSettings, bool enableNumbering, wxPoint aOrigPos )
: DIALOG_CREATE_ARRAY_BASE( aParent ), : 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_hSpacing( aParent, m_labelDx, m_entryDx, m_unitLabelDx ),
m_vSpacing( aParent, m_labelDy, m_entryDy, m_unitLabelDy ), m_vSpacing( aParent, m_labelDy, m_entryDy, m_unitLabelDy ),
m_hOffset( aParent, m_labelOffsetX, m_entryOffsetX, m_unitLabelOffsetX ), 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_vCentre( aParent, m_labelCentreY, m_entryCentreY, m_unitLabelCentreY ),
m_circRadius( aParent, m_labelCircRadius, m_valueCircRadius, m_unitLabelCircRadius ), m_circRadius( aParent, m_labelCircRadius, m_valueCircRadius, m_unitLabelCircRadius ),
m_circAngle( aParent, m_labelCircAngle, m_entryCircAngle, m_unitLabelCircAngle ), m_circAngle( aParent, m_labelCircAngle, m_entryCircAngle, m_unitLabelCircAngle ),
m_cfg_persister( saved_array_options.m_optionsSet ), m_cfg_persister( saved_array_options.m_optionsSet )
m_originalItemPosition( aOrigPos ),
m_numberingEnabled( enableNumbering )
{ {
// Set up numbering scheme drop downs character set strings // Set up numbering scheme drop downs character set strings
for( const auto& numData : numberingTypeData ) 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 ) void DIALOG_CREATE_ARRAY::OnParameterChanged( wxCommandEvent& event )
{ {
setControlEnablement(); setControlEnablement();
@ -297,13 +290,14 @@ static bool validateLongEntry( const wxTextEntry& entry, long& dest, const wxStr
bool DIALOG_CREATE_ARRAY::TransferDataFromWindow() bool DIALOG_CREATE_ARRAY::TransferDataFromWindow()
{ {
ARRAY_OPTIONS* newSettings = NULL; std::unique_ptr<ARRAY_OPTIONS> newSettings;
wxArrayString errors; wxArrayString errors;
const wxWindow* page = m_gridTypeNotebook->GetCurrentPage(); const wxWindow* page = m_gridTypeNotebook->GetCurrentPage();
if( page == m_gridPanel ) if( page == m_gridPanel )
{ {
ARRAY_GRID_OPTIONS* newGrid = new ARRAY_GRID_OPTIONS(); auto newGrid = std::make_unique<ARRAY_GRID_OPTIONS>();
bool ok = true; bool ok = true;
// ints // ints
@ -357,13 +351,11 @@ bool DIALOG_CREATE_ARRAY::TransferDataFromWindow()
// Only use settings if all values are good // Only use settings if all values are good
if( ok ) if( ok )
newSettings = newGrid; newSettings = std::move( newGrid );
else
delete newGrid;
} }
else if( page == m_circularPanel ) else if( page == m_circularPanel )
{ {
ARRAY_CIRCULAR_OPTIONS* newCirc = new ARRAY_CIRCULAR_OPTIONS(); auto newCirc = std::make_unique<ARRAY_CIRCULAR_OPTIONS>();
bool ok = true; bool ok = true;
newCirc->m_centre.x = m_hCentre.GetValue(); newCirc->m_centre.x = m_hCentre.GetValue();
@ -381,8 +373,9 @@ bool DIALOG_CREATE_ARRAY::TransferDataFromWindow()
if( newCirc->GetNumberingStartIsSpecified() ) if( newCirc->GetNumberingStartIsSpecified() )
{ {
ok = ok && validateNumberingTypeAndOffset( *m_entryCircNumberingStart, ok = ok
*m_choiceCircNumbering, newCirc->m_axis, errors ); && validateNumberingTypeAndOffset( *m_entryCircNumberingStart,
*m_choiceCircNumbering, newCirc->m_axis, errors );
} }
else else
{ {
@ -394,18 +387,16 @@ bool DIALOG_CREATE_ARRAY::TransferDataFromWindow()
// Only use settings if all values are good // Only use settings if all values are good
if( ok ) if( ok )
newSettings = newCirc; newSettings = std::move( newCirc );
else
delete newCirc;
} }
// If we got good settings, send them out and finish // If we got good settings, send them out and finish
if( newSettings ) if( newSettings )
{ {
delete m_settings;
// assign pointer and ownership here // assign pointer and ownership here
m_settings = newSettings; m_settings = std::move( newSettings );
// persist the control state for next time
m_cfg_persister.ReadConfigFromControls(); m_cfg_persister.ReadConfigFromControls();
return true; return true;

View File

@ -22,8 +22,8 @@
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA
*/ */
#ifndef __DIALOG_CREATE_ARRAY__ #ifndef DIALOG_CREATE_ARRAY__H_
#define __DIALOG_CREATE_ARRAY__ #define DIALOG_CREATE_ARRAY__H_
// Include the wxFormBuider header base: // Include the wxFormBuider header base:
#include <dialog_create_array_base.h> #include <dialog_create_array_base.h>
@ -32,53 +32,28 @@
#include <class_board_item.h> #include <class_board_item.h>
#include <pcb_base_frame.h> #include <pcb_base_frame.h>
#include <boost/bimap.hpp>
#include <widgets/unit_binder.h> #include <widgets/unit_binder.h>
#include <widgets/widget_save_restore.h> #include <widgets/widget_save_restore.h>
#include <memory>
class DIALOG_CREATE_ARRAY : public DIALOG_CREATE_ARRAY_BASE class DIALOG_CREATE_ARRAY : public DIALOG_CREATE_ARRAY_BASE
{ {
public: public:
/**
// Constructor and destructor * Construct a new dialog.
DIALOG_CREATE_ARRAY( PCB_BASE_FRAME* aParent, bool enableNumbering, *
wxPoint aOrigPos ); * @param aParent the parent window
* @param aOptions the options that will be re-seated when dialog is validly closed
~DIALOG_CREATE_ARRAY(); * @param aEnableNumbering enable pad numbering
* @param aOrigPos original item position (used for computing the circular array radius)
/*!
* @return the array options set by this dialogue, or NULL if they were
* not set, or could not be set
*/ */
ARRAY_OPTIONS* GetArrayOptions() const DIALOG_CREATE_ARRAY( PCB_BASE_FRAME* aParent, std::unique_ptr<ARRAY_OPTIONS>& aOptions,
{ bool enableNumbering, wxPoint aOrigPos );
return m_settings;
}
private: 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 // Event callbacks
void OnParameterChanged( wxCommandEvent& event ) override; void OnParameterChanged( wxCommandEvent& event ) override;
// Internal callback handlers // Internal callback handlers
void setControlEnablement(); void setControlEnablement();
@ -86,8 +61,26 @@ private:
bool TransferDataFromWindow() override; bool TransferDataFromWindow() override;
/**
* The settings to re-seat on dialog OK.
*/
std::unique_ptr<ARRAY_OPTIONS>& 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 // some uses of arrays might not allow component renumbering
bool m_numberingEnabled; 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_