kicad-developers team mailing list archive

[Lorenzo Marcantonio <l.marcantonio@xxxxxxxxxxxx>]

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.

> 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?)

> 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.

> 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).

> 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)

> 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

-- 
Lorenzo Marcantonio
Logos Srl