A while back, Jeff emailed me a note suggesting that KiCad was trying to
decouple from the wxWidgets type wxPoint except where needed in the UI
code. He suggested that I should use VECTOR2I instead.
I understand and support the desire to decouple from wxWidgets. However,
looking at the implementation of vector2d.h I was surprised to discover
that VECTOR2I is a simple array of /int/ like I would find in an old
Fortran program. It's not even a C structure, let alone a well-defined
C++ object.
One of the goals of object-oriented programming is to create descriptive
objects. Programmers can tell what an object represents by looking at
the name and implementation of the object. Well-crafted object help
avoid programming errors that could result from performing undefined
operations or using data from incompatible objects. In geometry, if we
subtract the 2d location of one point from the 2d location of another
point the result is a 2d vector describing the distance between these
points. However, this vector is not a point. In OOP this vector should
be a different object than a point.
The wxWidgets objects wxPoint and wxRealPoint are descriptively named,
though poorly implemented. Aside from where code abuses these, they
clearly represent locations. Replacing these with non-OOB typedefs such
as VECTOR2I removes the clarity that a variable is intended to represent
a location. The name is not descriptive of a location. There is no
sanity check to prevent performing a nonsensical operation like adding
two locations. It tells the programmer nothing about whether a variable
contains a location, a length, or just two loosely-related values.
Rather than replacing wxPoint with VECTOR2I, let us consider how we can
make our code more object-oriented rather than more Fortran-like. I
believe we should replace wxPoint and wxRealPoint with KiCad-specific
objects. These objects should be named such that they clearly indicate
that they of represent locations rather than a simple array of integer
values. They should be implemented such that nonsensical operations are
prohibited and sanity checks put in place where possible. For example,
attempting to add two locations should result in a compile-time error.
Additional objects should be defined to represent the delta between two
locations, and the operations of adding a delta to a location should
result in a location.
Just for discussion, let's assume the replacement for wxPoint is named
KiPoint. The result of subtracting two KiPoint objects would be another
object called KiDelta. Adding two KiPoint objects should be undefined,
and result in a compile-time error, as this is a nonsense operation.
Adding a KiPoint and a KiDelta should return a KiPoint. We should never
abuse a KiPoint to represent anything else, such as using the X value as
the radius of a circle as is currently done. I suspect most of the
places where this would be much more than a mechanical substitution is a
place where the code abuses the type or is a latent bug.
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.
-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
