On 6/22/19 10:52 AM, Seth Hillbrand wrote:
On 2019-06-21 22:54, Reece R. Pollack wrote:
Doing this now, before we go too far down the path of replacing
wxWidgets types with non-OOB arrays would enhance readability and make
the code more robust. Using VECTOR2I is going the wrong way.
Hi Reece-
Codebase cleaning like you suggest can go a long way toward improving
sustainability and readability. But done the wrong way, it will have
the opposite effect as we fight with return types that aren't fully
matched.
Before we decide to do this type of deep plumbing on the KiCad
innards, I'd love to see the proposed specification for what's being
implemented. Otherwise, this'll be a bike shedding discussion.
I've been in far too many "code reviews" where the focus became the
naming of variables and not on the algorithm being implemented. So I
agree with your comment in principle.
That said, the lack of type specificity already makes portions of our
code hard to maintain. The DIALOG_GRAPHIC_ITEM_PROPERTIES class already
reuses the m_endX UNIT_BINDER to represent both the X-axis endpoint of
an object and the radius of a circle. This requires special handling
when performing display origin transforms because the X-axis endpoint
requires origin transformation while the circle radius does not. This
illustrates the risk of treating everything as "just a pair of numbers"
because it's "easy".
I'm not suggesting rushing into a major change. But replacing a
descriptive object like wxPoint with a non-descriptive object like
VECTOR2I isn't an improvement. Especially when its implementation is
somewhat non-intuitive, as Hauptmech points out.
-Reece
_______________________________________________
Mailing list: https://launchpad.net/~kicad-developers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~kicad-developers
More help : https://help.launchpad.net/ListHelp
