SIM_MODEL code simplification.

1) Reduce API surface.
2) Prefer for(int) loops when what we're looking for is the index of
   the item to be found
3) Move "_" escape processing to SIM_MODEL_NGSPICE (which is the only
   model which uses the escape mechanism)

eeschema/sim/sim_model.cpp

@@ -532,18 +532,13 @@ std::string SIM_MODEL::GetFieldValue( const std::vector<T>* aFields, const std::
     if( !aFields )
         return ""; // Should not happen, T=void specialization will be called instead.
 
-    auto it = std::find_if( aFields->begin(), aFields->end(),
+    for( const T& field : *aFields )
-                            [aFieldName]( const T& field )
+    {
-                            {
+        if( field.GetName() == aFieldName )
-                                return field.GetName() == aFieldName;
+            return aResolve ? field.GetShownText().ToStdString() : field.GetText().ToStdString();
-                            } );
+    }
 
-    if( it == aFields->end() )
+    return "";
-        return "";
-    else if( aResolve )
-        return std::string( it->GetShownText().ToUTF8() );
-    else
-        return std::string( it->GetText().ToUTF8() );
 }
 
 
@@ -660,39 +655,30 @@ void SIM_MODEL::SetPinSymbolPinNumber( int aPinIndex, const std::string& aSymbol
 void SIM_MODEL::SetPinSymbolPinNumber( const std::string& aPinName,
                                        const std::string& aSymbolPinNumber )
 {
-    int aPinIndex = -1;
-
     const std::vector<std::reference_wrapper<const PIN>> pins = GetPins();
 
     for( int ii = 0; ii < (int) pins.size(); ++ii )
     {
         if( pins.at( ii ).get().name == aPinName )
         {
-            aPinIndex = ii;
+            SetPinSymbolPinNumber( ii, aSymbolPinNumber );
-            break;
+            return;
         }
     }
 
-    if( aPinIndex < 0 )
+    // If aPinName wasn't in fact a name, see if it's a raw (1-based) index.  This is required
-    {
+    // for legacy files which didn't use pin names.
-        // If aPinName wasn't in fact a name, see if it's a raw (1-based) index.  This is
+    int aPinIndex = (int) strtol( aPinName.c_str(), nullptr, 10 );
-        // required for legacy files which didn't use pin names.
-        aPinIndex = (int) strtol( aPinName.c_str(), nullptr, 10 );
 
-        // Convert to 0-based.  (Note that this will also convert the error state to -1, which
+    if( aPinIndex < 1 )
-        // means we don't have to check for it separately.)
-        aPinIndex--;
-    }
-
-    if( aPinIndex < 0 )
     {
-        THROW_IO_ERROR( wxString::Format( _( "Could not find a pin named '%s' in "
+        THROW_IO_ERROR( wxString::Format( _( "Could not find a pin named '%s' in simulation "
-                                             "simulation model of type '%s'" ),
+                                             "model of type '%s'" ),
                                           aPinName,
                                           GetTypeInfo().fieldValue ) );
     }
 
-    SetPinSymbolPinNumber( aPinIndex, aSymbolPinNumber );
+    SetPinSymbolPinNumber( --aPinIndex /* convert to 0-based */, aSymbolPinNumber );
 }
 
 
@@ -705,34 +691,27 @@ const SIM_MODEL::PARAM& SIM_MODEL::GetParam( unsigned aParamIndex ) const
 }
 
 
-const SIM_MODEL::PARAM* SIM_MODEL::FindParam( const std::string& aParamName ) const
+int SIM_MODEL::doFindParam( const std::string& aParamName ) const
 {
     std::string lowerParamName = boost::to_lower_copy( aParamName );
 
     std::vector<std::reference_wrapper<const PARAM>> params = GetParams();
 
-    auto it = std::find_if( params.begin(), params.end(),
+    for( int ii = 0; ii < (int) params.size(); ++ii )
-                            [lowerParamName]( const PARAM& param )
-                            {
-                                return param.info.name == lowerParamName;
-                            } );
-
-    // Also check for model params that had to be escaped due to collisions with instance
-    // params.
-    if( it == params.end() )
     {
-        it = std::find_if( params.begin(), params.end(),
+        if( params[ii].get().info.name == lowerParamName )
-                           [lowerParamName]( const PARAM& param )
+            return ii;
-                           {
-                               return param.info.name == lowerParamName + "_";
-                           } );
-
     }
 
-    if( it == params.end() )
+    return -1;
-        return nullptr;
+}
 
-    return &it->get();
+
+const SIM_MODEL::PARAM* SIM_MODEL::FindParam( const std::string& aParamName ) const
+{
+    int idx = doFindParam( aParamName );
+
+    return idx >= 0 ? &GetParam( idx ) : nullptr;
 }
 
 
@@ -776,48 +755,12 @@ void SIM_MODEL::SetParamValue( int aParamIndex, const std::string& aValue,
 }
 
 
-void SIM_MODEL::SetParamValue( const std::string& aParamName, const SIM_VALUE& aValue )
-{
-    std::string lowerParamName = boost::to_lower_copy( aParamName );
-
-    std::vector<std::reference_wrapper<const PARAM>> params = GetParams();
-
-    auto it = std::find_if( params.begin(), params.end(),
-                            [lowerParamName]( const PARAM& param )
-                            {
-                                return param.info.name == lowerParamName;
-                            } );
-
-    // Also check for model params that had to be escaped due to collisions with instance
-    // params.
-    if( it == params.end() )
-    {
-        it = std::find_if( params.begin(), params.end(),
-                           [lowerParamName]( const PARAM& param )
-                           {
-                               return param.info.name == lowerParamName + "_";
-                           } );
-
-    }
-
-    if( it == params.end() )
-    {
-        THROW_IO_ERROR( wxString::Format( _( "Could not find a parameter named '%s' in "
-                                             "simulation model of type '%s'" ),
-                                          aParamName,
-                                          GetTypeInfo().fieldValue ) );
-    }
-
-    SetParamValue( static_cast<int>( it - params.begin() ), aValue );
-}
-
-
 void SIM_MODEL::SetParamValue( const std::string& aParamName, const std::string& aValue,
                                SIM_VALUE::NOTATION aNotation )
 {
-    const PARAM* param = FindParam( aParamName );
+    int idx = doFindParam( aParamName );
 
-    if( !param )
+    if( idx < 0 )
     {
         THROW_IO_ERROR( wxString::Format( _( "Could not find a parameter named '%s' in "
                                              "simulation model of type '%s'" ),
@@ -825,8 +768,7 @@ void SIM_MODEL::SetParamValue( const std::string& aParamName, const std::string&
                                           GetTypeInfo().fieldValue ) );
     }
 
-    const SIM_VALUE& value = *FindParam( aParamName )->value;
+    SetParamValue( idx, aValue, aNotation );
-    SetParamValue( aParamName, *SIM_VALUE::Create( value.GetType(), aValue, aNotation ) );
 }
 
 

eeschema/sim/sim_model.h

@@ -495,7 +495,6 @@ public:
     virtual void SetParamValue( int aParamIndex, const SIM_VALUE& aValue );
     void SetParamValue( int aParamIndex, const std::string& aValue,
                         SIM_VALUE::NOTATION aNotation = SIM_VALUE::NOTATION::SI );
-    void SetParamValue( const std::string& aParamName, const SIM_VALUE& aValue );
     void SetParamValue( const std::string& aParamName, const std::string& aValue,
                         SIM_VALUE::NOTATION aNotation = SIM_VALUE::NOTATION::SI );
 
@@ -537,6 +536,8 @@ protected:
 
     virtual void CreatePins( const std::vector<LIB_PIN*>& aSymbolPins );
 
+    virtual int doFindParam( const std::string& aParamName ) const;
+
 private:
     template <typename T>
     void doReadDataFields( const std::vector<T>* aFields, const std::vector<LIB_PIN*>& aPins );

eeschema/sim/sim_model_ngspice.cpp

@@ -81,8 +81,28 @@ SIM_MODEL_NGSPICE::SIM_MODEL_NGSPICE( TYPE aType ) :
 }
 
 
+int SIM_MODEL_NGSPICE::doFindParam( const std::string& aParamName ) const
+{
+    // Special case to allow escaped model parameters (suffixed with "_")
+
+    std::string lowerParamName = boost::to_lower_copy( aParamName );
+
+    std::vector<std::reference_wrapper<const PARAM>> params = GetParams();
+
+    for( int ii = 0; ii < (int) params.size(); ++ii )
+    {
+        const PARAM& param = params[ii];
+
+        if( param.info.name == lowerParamName || param.info.name == lowerParamName + "_" )
+            return ii;
+    }
+
+    return -1;
+}
+
+
 void SIM_MODEL_NGSPICE::SetParamFromSpiceCode( const std::string& aParamName,
-                                               const std::string& aParamValue,
+                                               const std::string& aValue,
                                                SIM_VALUE_GRAMMAR::NOTATION aNotation )
 {
     std::string paramName = boost::to_lower_copy( aParamName );
@@ -99,67 +119,54 @@ void SIM_MODEL_NGSPICE::SetParamFromSpiceCode( const std::string& aParamName,
 
     std::vector<std::reference_wrapper<const PARAM>> params = GetParams();
 
-    auto paramIt = std::find_if( params.begin(), params.end(),
+    for( int ii = 0; ii < (int) params.size(); ++ii )
-                                 [lowerParamName]( const PARAM& param )
-                                 {
-                                      return !param.info.isSpiceInstanceParam
-                                          && param.info.category != PARAM::CATEGORY::SUPERFLUOUS
-                                          && ( param.info.name == lowerParamName
-                                               || param.info.name == lowerParamName + "_" );
-                                 } );
-
-    if( paramIt != params.end() )
     {
-        SIM_MODEL::SetParamValue( static_cast<int>( paramIt - params.begin() ), aParamValue, aNotation );
+        const PARAM& param = params[ii];
-        return;
-    }
 
+        if( !param.info.isSpiceInstanceParam
+            && param.info.category != PARAM::CATEGORY::SUPERFLUOUS
+            && ( param.info.name == lowerParamName || param.info.name == lowerParamName + "_" ) )
+        {
+            const SIM_VALUE& value = *GetParam( ii ).value;
+            SetParamValue( ii, *SIM_VALUE::Create( value.GetType(), aValue, aNotation ) );
+            return;
+        }
+    }
 
     // One Spice param can have multiple names, we need to take this into account.
 
-    std::vector<PARAM::INFO> ngspiceParams = ModelInfo( getModelType() ).modelParams;
+    // Now we search the base model parameters without excluding superfluous parameters (which
+    // may be aliases to non-superfluous parameters).
 
-    auto ngspiceParamIt = std::find_if( ngspiceParams.begin(), ngspiceParams.end(),
+    for( const PARAM::INFO& ngspiceParamInfo : ModelInfo( getModelType() ).modelParams )
-                                        [lowerParamName]( const PARAM& param )
-                                        {
-                                            // Now we search without excluding Spice instance
-                                            // parameters and superfluous parameters.
-                                            return param.info.name == lowerParamName;
-                                        } );
-
-    if( ngspiceParamIt == ngspiceParams.end() )
     {
-        if( canSilentlyIgnoreParam( paramName ) )
+        if( ngspiceParamInfo.name == lowerParamName )
-            return;
+        {
+            // Find an actual parameter with the same id.  Even if the ngspiceParam was
+            // superfluous, its alias target might not be.
+            for( int ii = 0; ii < (int) params.size(); ++ii )
+            {
+                const PARAM& param = params[ii];
 
-        THROW_IO_ERROR( wxString::Format( "Failed to set parameter '%s' to value '%s'",
+                if( param.info.id == ngspiceParamInfo.id
-                                          aParamName,
+                        && param.info.category != PARAM::CATEGORY::SUPERFLUOUS )
-                                          aParamValue ) );
+                {
+                    const SIM_VALUE& value = *GetParam( ii ).value;
+                    SetParamValue( ii, *SIM_VALUE::Create( value.GetType(), aValue, aNotation ) );
+                    return;
+                }
+            }
+
+            break;
+        }
     }
 
-
+    if( !canSilentlyIgnoreParam( paramName ) )
-    // We obtain the id of the Ngspice param that is to be set. We use this id to address the
-    // parameter to be set here because a superfluous parameter may be an alias: this will
-    // dereference it.
-    unsigned id = ngspiceParamIt->id;
-
-    // Find an actual parameter with the same id.
-    paramIt = std::find_if( params.begin(), params.end(),
-                            [id]( const PARAM& param )
-                            {
-                                // Look for any non-superfluous parameter with the same id.
-                                return param.info.id == id
-                                    && param.info.category != PARAM::CATEGORY::SUPERFLUOUS;
-                            } );
-    
-    if( paramIt == params.end() )
     {
         THROW_IO_ERROR( wxString::Format( "Failed to set parameter '%s' to value '%s'",
                                           aParamName,
-                                          aParamValue ) );
+                                          aValue ) );
     }
-
-    SIM_MODEL::SetParamValue( static_cast<int>( paramIt - params.begin() ), aParamValue, aNotation );
 }
 
 

eeschema/sim/sim_model_ngspice.h

@@ -46,7 +46,7 @@ public:
 
     SIM_MODEL_NGSPICE( TYPE aType );
 
-    void SetParamFromSpiceCode( const std::string& aParamName, const std::string& aParamValue,
+    void SetParamFromSpiceCode( const std::string& aParamName, const std::string& aValue,
                                 SIM_VALUE_GRAMMAR::NOTATION aNotation ) override;
 
     // Protected because it's accessed by QA tests.
@@ -104,6 +104,8 @@ protected:
 
     static const MODEL_INFO& ModelInfo( MODEL_TYPE aType );
 
+    int doFindParam( const std::string& aParamName ) const override;
+
 private:
     bool requiresSpiceModelLine() const override { return false; }