From 8cb8d55843aa57f4764590715a291acb800a4e69 Mon Sep 17 00:00:00 2001 From: Jon Evans Date: Sat, 7 Mar 2020 16:06:33 -0500 Subject: [PATCH] Remove unnecessary exception handling in settings Use optionals instead where a value can be absent Fixes #4024 --- common/settings/color_settings.cpp | 18 +++++------- common/settings/json_settings.cpp | 27 ++++++++++------- include/settings/color_settings.h | 8 ++--- include/settings/json_settings.h | 15 ++++++---- include/settings/parameters.h | 47 ++++++++++-------------------- 5 files changed, 50 insertions(+), 65 deletions(-) diff --git a/common/settings/color_settings.cpp b/common/settings/color_settings.cpp index fe2f56052e..fcd694e3fe 100644 --- a/common/settings/color_settings.cpp +++ b/common/settings/color_settings.cpp @@ -300,20 +300,16 @@ bool COLOR_SETTINGS::MigrateFromLegacy( wxConfigBase* aCfg ) COLOR4D COLOR_SETTINGS::GetColor( int aLayer ) const { - try + if( m_color_context == COLOR_CONTEXT::FOOTPRINT && aLayer >= PCBNEW_LAYER_ID_START + && aLayer <= GAL_LAYER_ID_END ) { - if( m_color_context == COLOR_CONTEXT::FOOTPRINT && aLayer >= PCBNEW_LAYER_ID_START - && aLayer <= GAL_LAYER_ID_END ) - { - aLayer += FPEDIT_LAYER_ID_START; - } + aLayer += FPEDIT_LAYER_ID_START; + } + if( m_colors.count( aLayer ) ) return m_colors.at( aLayer ); - } - catch( std::out_of_range& ) - { - return COLOR4D::UNSPECIFIED; - } + + return COLOR4D::UNSPECIFIED; } diff --git a/common/settings/json_settings.cpp b/common/settings/json_settings.cpp index 12b1b327d5..597a72c2a3 100644 --- a/common/settings/json_settings.cpp +++ b/common/settings/json_settings.cpp @@ -237,20 +237,22 @@ void JSON_SETTINGS::SaveToFile( const std::string& aDirectory ) } -nlohmann::json JSON_SETTINGS::GetJson( std::string aPath ) const +OPT JSON_SETTINGS::GetJson( std::string aPath ) const { - nlohmann::json ret( {} ); + nlohmann::json::json_pointer ptr = PointerFromString( std::move( aPath ) ); - // Will throw an exception if the path is not found - try - { - ret = this->at( PointerFromString( std::move( aPath ) ) ); - } - catch( ... ) + if( this->contains( ptr ) ) { + try + { + return OPT{ this->at( ptr ) }; + } + catch( ... ) + { + } } - return ret; + return OPT{}; } @@ -403,9 +405,12 @@ void JSON_SETTINGS::ReleaseNestedSettings( NESTED_SETTINGS* aSettings ) // Specializations to allow conversion between wxString and std::string via JSON_SETTINGS API -template<> wxString JSON_SETTINGS::Get( std::string aPath ) const +template<> OPT JSON_SETTINGS::Get( std::string aPath ) const { - return wxString( GetJson( std::move( aPath ) ).get().c_str(), wxConvUTF8 ); + if( OPT opt_json = GetJson( std::move( aPath ) ) ) + return wxString( opt_json->get().c_str(), wxConvUTF8 ); + + return NULLOPT; } diff --git a/include/settings/color_settings.h b/include/settings/color_settings.h index 62e1930e21..bff375615f 100644 --- a/include/settings/color_settings.h +++ b/include/settings/color_settings.h @@ -129,12 +129,8 @@ public: COLOR4D val = m_default; - try - { - val = aSettings->Get( m_path ); - } - catch( ... ) - {} + if( OPT optval = aSettings->Get( m_path ) ) + val = *optval; ( *m_map )[ m_key ] = val; } diff --git a/include/settings/json_settings.h b/include/settings/json_settings.h index bf0f78c54c..7a54d7dadf 100644 --- a/include/settings/json_settings.h +++ b/include/settings/json_settings.h @@ -25,6 +25,8 @@ #include #include +#include + class wxConfigBase; class NESTED_SETTINGS; class PARAM_BASE; @@ -90,19 +92,22 @@ public: * @param aPath is a string containing one or more keys separated by '.' * @return a JSON object from within this one */ - nlohmann::json GetJson( std::string aPath ) const; + OPT GetJson( std::string aPath ) const; /** * Fetches a value from within the JSON document. - * Will throw an exception if the value is not found or a mismatching type. + * Will return an empty optional if the value is not found or a mismatching type. * @tparam ValueType is the type to cast to * @param aPath is the path within the document to retrieve * @return a value from within this document */ template - ValueType Get( std::string aPath ) const + OPT Get( std::string aPath ) const { - return GetJson( std::move( aPath ) ).get(); + if( OPT ret = GetJson( std::move( aPath ) ) ) + return ret->get(); + + return NULLOPT; } /** @@ -220,7 +225,7 @@ protected: // Specializations to allow conversion between wxString and std::string via JSON_SETTINGS API -template<> wxString JSON_SETTINGS::Get( std::string aPath ) const; +template<> OPT JSON_SETTINGS::Get( std::string aPath ) const; template<> void JSON_SETTINGS::Set( std::string aPath, wxString aVal ); diff --git a/include/settings/parameters.h b/include/settings/parameters.h index ae95c5c984..224efe691c 100644 --- a/include/settings/parameters.h +++ b/include/settings/parameters.h @@ -22,10 +22,11 @@ #define _PARAMETERS_H #include -#include #include #include -#include "json_settings.h" + +#include +#include class PARAM_BASE @@ -81,9 +82,9 @@ public: ValueType val = m_default; - try + if( OPT optval = aSettings->Get( m_path ) ) { - val = aSettings->Get( m_path ); + val = *optval; if( m_use_minmax ) { @@ -91,8 +92,6 @@ public: val = m_default; } } - catch( ... ) - {} *m_ptr = val; } @@ -141,16 +140,10 @@ public: ValueType val = m_default; - try - { - if( std::is_same::value ) - val = aSettings->GetJson( m_path ); - else - val = aSettings->Get( m_path ); - } - catch( ... ) - { - } + if( std::is_same::value ) + val = aSettings->GetJson( m_path ); + else + val = aSettings->Get( m_path ); m_setter( val ); } @@ -274,21 +267,16 @@ public: std::vector val = m_default; - try + if( OPT js = aSettings->GetJson( m_path ) ) { - nlohmann::json js = aSettings->GetJson( m_path ); - - if( js.is_array() ) + if( js->is_array() ) { val.clear(); - for( const auto& el : js.items() ) + for( const auto& el : js->items() ) val.push_back( el.value().get() ); } } - catch( ... ) - { - } *m_ptr = val; } @@ -342,21 +330,16 @@ public: std::map val = m_default; - try + if( OPT js = aSettings->GetJson( m_path ) ) { - nlohmann::json js = aSettings->GetJson( m_path ); - - if( js.is_object() ) + if( js->is_object() ) { val.clear(); - for( const auto& el : js.items() ) + for( const auto& el : js->items() ) val[ el.key() ] = el.value().get(); } } - catch( ... ) - { - } *m_ptr = val; }