SpiceSharp / SpiceSharpParser

SPICE netlists parser for .NET
MIT License
25 stars 6 forks source link

Rename SpiceParserResult SpiceSharpModel Property and Type #145

Closed ToddThomson closed 3 years ago

ToddThomson commented 4 years ago

The SpiceParserResult returned from Parser.ParseNetlist() returns the property: SpiceSharpModel of Type SpiceNetlistReaderResult. Perhaps the naming here could be improved. SpiceSharpModel is accurate, but does not need to contain your Spice flavour (sorry). Perhaps you could rename SpiceNetlistReaderResult to SpiceModel, as that is what it is, and then rename the property to SpiceModel.

marcin-golebiowski commented 4 years ago

Take a look at: https://github.com/SpiceSharp/SpiceSharpParser/blob/master/src/SpiceSharpParser/SpiceParserResult.cs

There are 3 properties: 2 with models (before preprocessing and after) and 1 with SpiceSharp model.

marcin-golebiowski commented 4 years ago

I'm open for any suggestions how to make it better :)

ToddThomson commented 4 years ago

@marcin-golebiowski Yes, I was reviewing the SpiceParserResult class. There is no issue with the properties themselves. I just thought the property name SpiceSharpModel should not contain your implementation specifics (SpiceSharp) and that the property Type was not accurately named ( a result within a result, but what is actually returned is a SpiceModel ). This is just my opinion/input. Thanks Marcin!

marcin-golebiowski commented 4 years ago

Please review #146

I think it's small step in right direction. I think there should be additional layer of abstraction before creating instances of Spice# library. For example there should be generic Resistor class etc.

What do you think?

marcin-golebiowski commented 4 years ago

ETA for this layer of abstraction: second week of 2020.

ToddThomson commented 4 years ago

@marcin-golebiowski Yes to Generics, Interfaces ( and async methods ).

I haven't got to the point of running circuit simulations, but my gut feel is that the API should be async.

marcin-golebiowski commented 4 years ago

Let me think about it. I like the idea :)

marcin-golebiowski commented 3 years ago

Changed by #152. I've simplified the naming and types.

Feel free to reopen the ticket if you disagree with the work.