kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxxxx>]

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
> > coupling.
>
> I prefer proxy functions (when there is no state) but the idea is that,
> exactly.
>
> > 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
> > interface.
>
> 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.