pcb

Comment 11 for bug 929123

Revision history for this message
Bert Timmerman (bert-timmerman) wrote :

Hi Charles,

I can agree to "Absolute" becoming boolean (#10), what to do with the value stored in Absolute ?

Looking in ChangeViaClearSize() I see:

Coord value = (Absolute) ? Absolute : Via->Clearance + Delta;

where "Absolute" does a binary switch (if, then, else) already.

This would become something like:

Coord value = absolute ? new_size_var : Via->Clearance + Delta;

Needs to be documented to give a concise representation of the "why", "what" and "how" of the new implementation and behaviour (with doxygen comments, and the User Manual if we break current behaviour).

See the doxygen comment for the function, it would surprise me if the return value is "TRUE" ... ever !

I think it should be a pointer value of PinType (a copy-paste error in the comment I assume, which at the time of doxygenation was overlooked by me).

#9 (above) confuses me: I think I read that sometimes this "Absolute" value is mis-used? for detecting the value originates from keystrokes. This sounds as overloading to me.

When "Absolute" becomes a boolean it should be just for the purpose of the binary [absolute, relative].

And what members would a "size delta" struct contain ?

If there needs to be another boolean "Keystroke" to pass on the information that the value originated from keystrokes ... make it so.

If it becomes to cumbersome to pass all these booleans from function to function, than an int with bit-fields containing them may be a solution ... more stuff to put in documentation.

Bottom line: please keep it as simple as possible ... and document the code ... if I misunderstood, please elaborate with code ;-)

Kind regards,

Bert Timmerman.