kicad-developers team mailing list archive

[Brian <lotharyx@xxxxxxxxx>]

Wayne,

> What exactly are you serializing to and from?  If it's just a string
> saved to a loaded from a database, why not just add public methods to
> the plugin object to serialize to and from a string?
My goal is to have my database contain legacy-format symbol data (since 
the new format doesn't exist in code yet -- my goal is not to roll my 
own format or implement a different one, but only to divert what would 
be in a file into a database instead).  I chose to encapsulate the way I 
did because:

1. SCH_LEGACY_PLUGIN_CACHE has no public interface in any header file; 
it is declared and defined solely within sch_legacy_plugin.cpp.  (There 
is only a forward declaration in sch_legacy_plugin.h)  To do as you 
suggest, I would have to write an entire, correct declaration for 
SCH_LEGACY_PLUGIN_CACHE that my plugin code could see.  That seems like 
a lot of extra work just to share a small subset of the functionality.
2. My plugin doesn't need any other part of SCH_LEGACY_PLUGIN_CACHE or 
SCH_LEGACY_PLUGIN, as it only handles symbols and not entire schematics 
and has its own caching scheme.
3. All of the actual serialization code could be static, but is defined 
as member methods on SCH_LEGACY_PLUGIN_CACHE (the only actual member 
variable dependency is version number, which is easily passed as an 
argument).  I don't want my plugin to have to instantiate an entire 
SCH_LEGACY_PLUGIN_CACHE just to turn a LIB_PART to/from a string.  Ergo, 
LEGACY_PART_READER / LEGACY_PART_WRITER are entirely static.
4. I could have just copied the entirety of the to/from string code from 
SCH_LEGACY_PLUGIN_CACHE into my plugin's code, but that seemed like the 
wrong thing to do for several reasons; code reuse being the primary.

> That's how we
> handled this in the board plugin.  I would also prefer to leave the
> parsing and formatting code in the current file.  The formatter part of
> this is going to go away most likely during v7 development but possibly
> during v6 development depending upon how fast I can get the new file
> format implemented.
A rebuttal:

I believe this is a way in which our underlying coding philosophies 
differ.  I am a fan of tight encapsulation and small-as-practical .cpp 
files.  While I will occasionally embed trivially-small helper classes 
in the headers/implementations of other classes, I generally tend to 
take a header-and-implementation-per-class approach.  The entirety of 
EESCHEMA could live in a single .cpp file, or every function definition 
could be in a separate file.  Beyond the requirements of the language, 
how code is divided into files is a religious argument.  I'm happy to 
defer to your seniority, but I wanted to explain the reason I made the 
choices I made.

As an aside, having my modifications removing large swaths of 
planned-obsolete code from sch_legacy_plugin.cpp seems like it would 
make your job easier when it comes time to eradicate it from the plugin; 
I've already sifted through and yanked out all of the helper methods.

As a second aside, having the read/write code for the new s-expression 
format similarly encapsulated would be awfully nice, so that I can 
change my ODBC plugin to use it when it's published, without having to 
go on another refactoring mission, and maintain all the same points as 
mentioned above.

> Also, please do not force the GCC ABI version in the CMake config.  You
> should be passing -DCMAKE_C_FLAGS="-fabi-version=2" and/or
> -DCMAKE_CXX_FLAGS="-fabi-version=2" on the command line when you call
> CMake to generate the build files if you need a specific GCC ABI version
> or any other personal compiler flags.  This kind of thing can cause all
> kind of headaches for developers so it's frowned upon by most projects.
Understood.  On that, I have no argument!

Kind regards,
-Brian
> Cheers,
>
> Wayne
>
> On 3/13/2019 12:16 PM, Brian wrote:
<snip>