← Back to team overview

kicad-developers team mailing list archive

Re: Plot and drill file generation via scripts

 

On 5/3/2013 5:04 PM, Lorenzo Marcantonio wrote:
On Fri, May 03, 2013 at 02:45:01PM -0500, Dick Hollenbeck wrote:
If they are not in one class, this makes the mix-in tougher back up at the PCB_BASE_FRAME.
You end up with 2,3,4 classes, each holding a BOARD*, and each being mixed in via multiple
inheritance?  Instead of one class.

I also said I didn't like the forceful mixin, in fact. I better see the
board actions as a separate functions rather than a class.

The dangers of separate functions is that developers (I include myself in that group) tend to not be disciplined enough to resist the temptation to couple objects directly inside the function or indirectly by calling other functions. This seems particularly true for UI objects. The reason is that it is easier and faster to couple them than it is to write the extra code to use a proxy or 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.

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

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.


I am thinking that multiple inheritance at the FRAME level is one line of code to do the
mixin.  And yes, of course it can live detached as well.  One class, two living
environments.  If instantiated stand alone, then it is an interface for python.  If
instantiated as part of frame, it augments a frame.

Why do you need to *instantiate* procedural code? Passing boards around
is easier, especially since the plan is to have many boards in the same
core IIRC. Also that would have a *huge* interface, not exactly easy to
maintain.

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 fits into the wxWidgets framework, take a look at the doodle sample from chapter 19 of the wxWidgets book at:

http://www.wxwidgets.org/docs/book/#examples

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


Since it is ONLY actions, i.e. procedural calls, then the integration happens in the
frame, no that is already done.  Just move *procedural* functions into BOARD_ACTIONS, no
event handling, no dialogs, no UI.

So no needed data members. In fact no class, only procedural code.
Object = data with operation on it. These are only operations on data
somewhere else, so no object (no polymorphism needed either).
A namespace if you want.  Exception: long (i.e. many calls needed
actions) would need a support class to keep the state.

Example 1: case for moving a track (only to give an idea, of course);
let's say that for some reason you can't simply do a TRACK::SetPosition
(everything invented!), so you need support from the board or whatever.
Or maybe it should only succeed depending on some DRC check, I don't
know.

Simple interface function: BA_MoveTrack and parameters (board,
track whatever). Nothing to instantiate, just call it. Putting it as
a mixin would only gain omission of the board pointer. If calling from
the frame it would be BA_MoveTrack( GetBoard(), stuff ) instead of
BA_MoveTrack( stuff ). Too marginal for a so big mixin.

From the outside it would be BA_MoveTrack( GetBoard(), stuff )
(GetBoard() obviously could be a local or member variable).
Otherwise you would have to a) allocate a whole object for BA (passing
the board) and b) calling each single action from that object.
So: ba = new BoardAction(GetBoard()); ba.MoveTrack(stuff); instead of BA_MoveTrack(GetBoard(), stuff); what do you gain interposing a class?

Example 2: the plot controller. It has state between calls so it need
members to remember it. Instantiate a controller, set options, plot,
close delete controller. Want to do *two* plots in parallel? No problem,
instantiate two of them, it should work. It's not an object in it's
'purest' sense but we don't have closures, so it will have to do.

Even the example 1 could be 'forced' to work as a class (but why?) if
you really wanted.

BA_TrackMover trackmover( GetBoard() );
trackmover.do(stuff);

That's exactly a command pattern. Runtime cost is nearly zero if on the
stack (since it only keeps a board). But why should I create a class
when a function suffice?

What am I missing?

That board actions don't need state in most of the cases and that it
don't need to be mixed in to the frame (PCB_FRAME IS-A BOARD_ACTIONS?
I don't think so...). And, in fact, even without the mixin you already
have access to the board.

The dialogs can stuff something like the PLUGIN PROPERTIES object, which could be renamed
to say, OPTIONS.  Then OPTIONS can be passed to some of the functions in BOARD_ACTIONS.
Or python can stuff OPTIONS, and do the same.

Right. In the dialog (let's say "save as") do something like:

OPTIONS opt; // Stuffed with stuff
BA_SaveAs( GetBoard(), &opt );

From scripting no differences.

This GetBoard would go to the frame or access some member in the dialog,
it doesn't matter. Otherwise, using a pseudo command pattern:

OPTIONS opt; // Stuffed with stuff
BA_BoardSaver saver( GetBoard() );
saver.SaveAs(&opt)

That's not exactly a command pattern: these 'command classes' would
have related function. The BA_BoardSaver could have Save and SaveAs, for
example. A true command pattern would be more like this:

OPTIONS opt;
BA_BoardSaveAsCmd saver( GetBoard() );
saver.SetOptions(&opt);
saver.execute(); // This would be polymorphic across commands

However, one class for each command is a PITA and it's not warranted
if you don't need the features of command (like serializable undo). As
I said you don't need a class for these things.

This has been my vision for 2 years.  Just saying you don't like it or that it is not
necessary, is actually an insufficient argument.  Will it work, and what is better if
anything, and why?

Yes it would work. My argument is: does it need to be a) a huge class
and b) a class at all when since it's mostly procedural code there is
no state to maintain? the only advantage I see in a mixin is not having
to pass the board to calls. Disadvantages are: the huge interface gives
huge recompilations; you can't keep separate 'transactions' in parallel, it
the cases where there actually is state to be kept; the state for *all*
the operations is put in one big class (so the BOARD_ACTIONS class would
have the state for plotting, near the state for drilling, near the state
for whatever and every else operation needs state). In short it becomes
almost a parallel global namespace (or more like a big union, since you
will not use every state as the same time). I'd say it's almost the
opposite of modular programming.

I discovered there is even a name for that: the blob :D
http://www.cs.iastate.edu/~hridesh/teaching/362/07/01/notes/antipatterns.pdf
From slide 16. And since it's an OO design class the idea 'just use
functions' is not contemplated (in Java you couldn't do it anyway)
Here is explained better: http://sourcemaking.com/antipatterns/the-blob
I'm am not against OO (not always, at least) but I think than functional
and procedural have their use.

With my idea you have: no classes when states is not needed, the
function/operation classes can be light and divided between files
without problem (so faster recompilation), every 'group' of operation
with state has its own little state (and it's independant with other
operations and even other instances of the same operation). You have to
pass the board at each call, but for these advantages I think is
acceptable (also: it would be needed anyway for between-board
operations).

Of course the 'best' solution would be finding the right home for each
operation, if possible (asking the track to move itself, for example:
but it would have to find it's own board and so on, it's not trivial)

These are the reason for which I think it would work better. Have
I forgot something?




Follow ups

References