← Back to team overview

kicad-developers team mailing list archive

Re: Plot and drill file generation via scripts


On 5/4/2013 12:57 PM, Lorenzo Marcantonio wrote:
On Sat, May 04, 2013 at 12:22:55PM -0400, Wayne Stambaugh wrote:
gate keeping object to prevent coupling.  We will never be able to
factor out the board (or schematic) code into a dso/dll until we
start being disciplined enough to prevent this kind of coupling.  I
think this really gets to the heart of the matter of what we as a
team should be trying to resolve.

Well, that's a policy issue... instead of separate function he proposed
functions in another class, so the temptation to do the thing 'here and
now' remains. The separation of concerns would be always a little
subjective: is this only for processing into the dialog or can be useful
for the script layer too? The risk is bloating the proxy classes.

The advantage of the proxy object is that the bloat and/or coupling is contained within the object which tends to make it more visible.

I just took a quit look at pcbplot.cpp and I already see a problem.
The separate function EnsureOuputDirectory() takes a pointer to a
wxTextCtrl.  It is called from within the PLOT_CONTROLLER object
function OpenPlotFile() which means that the PLOT_CONTROLLER object
cannot be compiled and linked without including the wxWidgets UI
code. I'm not trying to single out anybody but this kind of
procedural code exists throughout KiCad.  That being said, using a

Yes, I know that. During the plotting work I decoupled some things and
not other and I probably didn't even knew about the REPORTER stuff. In
fact that function should not be in the pcbplot file but somewhere else
(do we have something for 'file utilities', maybe?). It is a known bug
that that call could silently fail:(( give me a way to kick back an
exception or something to both python and the rest of pcbnew and I'll
fix it, however I don't think there is an infrastructure for it (or does
it exists?)

The REPORTER is brand new. It was Dick's idea as way to hide writing strings to a wxTextCtrl or any other object for that matter. I wrote it so I wouldn't introduce coupling the BOARD object to UI objects as part of the NETLIST_READER changes. Feel free to extend it and derive from it to suit your needs.

proxy object does not guarantee that coupling cannot happen within
the proxy object.  However, if your writing a proxy object you
should be aware that one of the goals of this object is to prevent

I prefer proxy functions (when there is no state) but the idea is that,

A simple solution for this problem above is to use the REPORTER
object in place of the wxTextCtrl object to hide the UI access.
This way the PLOT_CONTROLLER object could be built without any
direct knowledge of wxTextCtrl.  This solution is not as elegant or
robust as using an action object but it does solve UI object
coupling problem.

Wait a minute, I have to look at the code:D Ah OK, I remembered wrong,
it's used for the output messages. Yes, I agree that something like the
reporter object would be the right thing to use here.

This case was easy to find. The more insidious case is were it's buried two or three functions calls down stack. You can easily couple objects this way without even realizing it.

I don't see the need for a board action object to have a huge
interface.  Take a look at the wxCommand object.  This object is
conceptually similar to Dick's proposed ACTION object and only has
four functions in it's interface.  If you want to see how wxCommand

*If* the action object is like that, I agree with Dick, is a command
pattern (I did a similar example, I didn't know wx already had that).
The command need state do ensure undoability, so it's correctly some
kind of object; since our undo implementation works in another way,
*most* command don't need an implementation.

However Dick said this:

DICK SAID> If they are not in one class, this makes the mix-in tougher
DICK SAID> back up at the PCB_BASE_FRAME.  You end up with 2,3,4
DICK SAID> classes, each holding a BOARD*, and each being mixed in via
DICK SAID> multiple inheritance?  Instead of one class.

This make me think (perhaps incorrectly!) that he meant to do a whole
huge blob class containing all the actions for a board and mix it in
just to not having to pass a board pointer. If that's not his idea,
please excuse me. At the time I tought that the mixin was a strategy to
'ease' migrating things from the frame to the action classes (in
a properly designed system the mixin shouldn't be needed).

I'm not sure what Dick's intentions are here but I doubt he is talking about one huge object with all of the actions built into it unless it's something like the PAINTER object currently being use as part of the new rendering design. Perhaps the mixin is more of an action manager rather the actions themselves.

This simple object is used as a proxy to decouple the document (in
our case the BOARD) from the view (UI elements).  There doesn't have
to be view to use this object so it can safely be instantiated and
called from the command line or from scripting languages when the UI
is not present.  If we were using the wxWidgets doc/view framework,

Agree 100%, I already used similar approaches (both in OO and in
functional code) and it works fine (in our line of work we do *a lot*
of RPC thru various communication channels, so the transition from
a command from a serial port to a command object is very natural).
Having to derive a class for each command is a little nuisance so, if
possible, I'd like to avoid the 'pure' command pattern. If you decide on
a command class hierarchy however I have nothing against it, it's just
my personal preference (C++11 seems to support some kind of closure, but
probably the world is not ready for that:D)

I prefer a larger number small, self contained, easy to understand commands to a smaller number of large combined commands. My reasoning is modularity. Let's say we were already using commands (or actions or whatever you prefer to call them) and a draw trace command existed that contained all of our current code for drawing a trace. If someone comes along later with a new push-shove or an impedance matching draw trace command, we can easily support all of the draw trace commands within the same framework by substituting the appropriate command as long as these commands only draw traces. As soon as you start combining other trace operations, you make command substitution much more complex.

wxCommand would by us do/undo support with having to write our own
do/undo framework.  I am not suggesting we convert to the wxWidgets
doc/view framework or use wxCommand.  I am just using this as an
example of how decoupling it can be achieved without a huge object

If you have a Save object/function, a Load object/function, a MoveTrack
object/function and so on I agree it's a good idea. However from Dick's
mail I had the impression he was proposing something else, like a big
class containing *all* of these methods (again I'm excusing for the
misunderstanding, if that's the case).

Probably we are each one just proposing some variation of the same
technique and there is only a communication gap:D

I would not surprised if this were the case. I'm am coming to the conclusion that email is not the most effective way to communicate ideas. I find that code examples (either my own or a link to an example) do a better job of explaining my ideas than several paragraphs of detailed explanations.

Follow ups