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 : kicad-developers@xxxxxxxxxxxxxxxxxxx
Unsubscribe : https://launchpad.net/~kicad-developers
More help : https://help.launchpad.net/ListHelp
