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 _______________________________________________ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp