kicad-developers team mailing list archive

[Lorenzo Marcantonio <l.marcantonio@xxxxxxxxxxxx>]

On Fri, Apr 19, 2013 at 07:11:48PM -0500, Dick Hollenbeck wrote:

> LAYER_NUM is mostly used as an iterator in our code, used to walk through for loops.

Also as a parameter for indexing into stuff and especially LAYER_MSK
stuff. The main 'practical' reason for its introduction was finding
*every* place where a layer number was used.

> What an enum brings is range safety.  Used alone, it would only bring about a 1 in 28
> layer chance of providing the proper value.  This is your "type safety".  "Type safety" is
> a comforting phrase, and even to the point of being misleading when used with enums.  With
> enums it is essentially "fools gold", because it simply brackets a range of values.

Not only that. You *can't* do operations with enum without casting; in
fact the new enum classes in C++11 almost removes the automatic int
promotion. So the type safety here is: you can't pass another int as
a LAYER_NUM and you can't (theorically, but current C++ doesn't enforce
it) us a LAYER_NUM as an int. Pascal got them better, I agree (you had
the enumerate them with prev and succ)

The 'real' C++ solution would have been a class (something like an
iterator), but probably that would have been overweight.

> What would be real gold is "value accuracy".  Value accuracy cannot be achieved by enums
> alone.  "Value accuracy" comes from testing, unit tests, code maturity, asserts, proper
> limits on for loops, and other techniques.  In our case code maturity and for loop limits
> play a key role.

Let me check if I got the concept: "value accuracy" == the value is what
I wanted to represent and not something else unrelated. Example: it's
copper and not some other thing?

> So for a 1 in 28 chance guarantee, then why is it to be weighed so heavily as to promoted
> as a solution to anything?  It is *not* a solution to "value accuracy".  If you like fools
> gold, and it makes you feel good, there may be some merit for it.

First of all it avoids about 2^32-28 invalid values:D also without it
some things like the encoding in vias or a layer mask passed as a layer
number couldn't be caught by the compiler. *That* is type safety for me
(in statically typed languages like C++);

In the same way I claim as fool's gold trivial member accessors (if you
have Get and Set on a member, then it's the same as making it public,
it's easy to see)

> The downside, or the cost, of using LAYER_NUM is that it becomes the center of attention
> of every for() loop.  It makes the for() loop look non-standard, and makes opaque an
> iterator that really is nothing other than an index most of the time.  In fact, nearly all
> the time.

It seems to me that would be *exactly* what C++ standard practice is:

for (std::vector<STUFF>::iterator it = v.begin(); it != v.end(); ++it) {
    // Do stuff with *it
}

versus

for(LAYER_NUM it = FIRST_LAYER; it < LAST_LAYER; ++it) {
    // Do stuff with xx[it]
}

As I said before LAYER_NUM would have been theorically something like an
iterator class, but even for me that would have been too much.

> The upside is slim to none.  The downside is only moderate.

OK, quiz: what does this function want?

void stuff(int aLayer);

A layer number, a layer mask or some index from a dialog list? 

> The designers of Java go so sick of Microsoft's obfuscation of integer types, that they
> basically made it impossible to typedef an int.   Their thinking is to be seen as
> revolutionary, and simplifying.  Look at Java code, go back to the 80's and 90's and look
> at Microsoft Windows code examples.  Java is simple and elegant by comparison.  That is

Uhm... I don't agree with that... I learnt Windows API on the Petzhold
and (other than their hungarian notation) I didn't find them
obfuscated... even opengl has GLdouble and GLfloat (for portability
reason, here).

And I also think that java designers should be hung because they got
wrong almost every language feature :D

> because in part, "int" does not become the center of attention in every for loop.  The for
> loop limits, as I have said, are far more important to giving you "value accuracy".

> And int is more readable.

Not when you're trying to know what it's representing.

Another thing: your little man doing stuff on the stairs i.e. it has not
to be an int! I already knew that something 'strange' would be needed
for the new mask, but maybe it would be needed for the index too.

Let say (it's hypotetical, but could be a useful idea) that the new
layer design calls for non-enumerated layers; arguments for this were
already been given by Wayne for the paper size, for example. Also he
said we needed (possibly good) ideas, so I have a shot at it.
Brainstorming if you want.

Let say that we want the user to define whatever layers he want (one of
the CERN project, after all) and (for some reason, maybe for integration
for other files? I don't know) the layer identifier is not anymore an
int. For example, I find 'ugly' than on a two sided board I have to step
over 14 unused inner layer to get from front to bottom and also maybe
someone could be so crazy to want to do a 24-layers board on kicad. It
seems to me a plausible use case.

Now, I would design (in C++) this thing as follows: one layer container
for containing sets of layers (class LayerSet) and one layer descriptor
for each layer insided (class LayerDescriptor). Something like this:

class LayerDescriptor
{
    string system_name;
    string user_name;
    LayerSet *owner;
    // stuff
};

class LayerSet
{
    vector<LayerDescriptor*> layers;
    LAYER_NUM GetFirstCopperLayer() const;
    LAYER_NUM GetEndOfCopperLayers() const;
    // stuff
};

class LAYER_NUM
{
    LayerDescriptor *layer;
};

// adequate operator definitions to make LAYER_NUM work as a layer index
// and of course arrays using a layer number as index would have to be
// changed to maps or hash tables (surprisingly few!)

more or less, it's very sketchy... the idea is the
following: since the layer number (as in position) is not fixed anymore
(example: the user decided to add more inner layer, so all the layers
would be shifted), having to scan the whole database to fix the layer
number would be a massive PITA (could be a legitimate implementation to
keep LAYER_NUM an int, anyway).  Otherwise you could add the new inner
layer at the end but then copper layer numbers wouldn't be contiguous
and the usual loops would fail. From an efficiency standpoint it
shouldn't be *too* heavy, since LAYER_NUM doesn't need a vtable and is
more or less a pointer...

So it would work in this way: layer descriptors would live and be the
token identifying the layers (i.e. with immutable address). The layer
setup would properly configure the descriptor table and the operators
would use it (thru the owner pointer) to go forward, back or whatever.

Layer classification could be left the same or implemented as methods:

IsCopperLayer(layer) vs layer.IsCopperLayer()

one global function versus one LAYER_NUM method... I think is a style
issue (this is one of the things I don't like in message based OOP)

Layer iteration: from

for (LAYER_NUM i = FIRST_COPPER_LAYER; i < NB_COPPER_LAYERS; ++i)

to

for (LAYER_NUM i = ls.GetFirstCopperLayer();
     i != ls.GetEndOfCopperLayers();
     ++i)

Two things here: first there is need to access the layer set because we
need the layers of *that* board/module/whatever; second it the test
using != because order comparison could be expensive (the layer table
would have to be searched twice). And anyway it's the standard design
for C++ iterators.

I think that (even if as a rough idea) demonstrates how having an
abstraction for the 'layer index' concept would be useful.

So, no, I'm still convinced that not calling a LAYER_NUM an int is
a good idea, even if it's only to look for every place where a layer
number is used...I have done that *a lot*. Try grepping for int instead
:D

-- 
Lorenzo Marcantonio
Logos Srl