On Aug 1, 2013, at 10:34 AM, Johan Engelen wrote:
By the way, the "SP" of "SPPoint" is something we want to avoid for any new classes. That stands for "SodiPodi" which was the project Inkscape was forked from years ago.
The SP prefix should be kept for the time being, for the sake of consistency. Removing it should be a separate task.
Yes... I was explicitly calling out to not *add* to new classes (hence my inclusion of "for any new classes")... not for removing it if it already was present.
And Krzysztof was explicitly calling out to *add* it to new classes that have a similar function as the other SPFriends, with which I fully agree.
Right, and the project has discussed this a few times in the past, including discussions with Bryce, Ted, etc. As far as consistency goes, remember "A foolish consistency is the hobgoblin of little minds".
In this case, we had explicitly called out to no longer add the "SP" prefix even in areas that already had it. Adding in more instances will just make it that much harder to remove. We were called on to at least not make the problem worse.
That does lead us to a good opportunity in this case. Since if we drop the "sp" of the new name, that will leave only "Point". Now we end up with a class name that is fairly ambiguous and really needs some clarification. lib2geom has it's own 'point', and there are others. If we put back the "SP" prefix, all that really says is "the point class that happens to live in the SodiPodi codebase". Definitely not too helpful.
Reading the header indicates from the comment that this is for the SVG draft on connectors. In that draft, we have the explicit element named "point". Since Sebastian gave us such good info, we can use that to help the code. A verbose name could be "ConnectorPoint" or "PointElement". However in this case we can probably gain from the spec and since it is actually proposed as part of SVG itself we can call it "SVGPoint".
Taken in context, this would look like
SVGPoint *point = FooGetPoint(); ... Geom::Point where = point->getPosition();
*or* we could actually take advantage of consistency *and* C++ and go with a namespace (typing that previous line was a good hint to that possibility):
SVG::Point *point = FooGetPoint(); ... Geom::Point where = point->getPosition();
So that changes from SPPoint which says "The point class in the SodiPodi codebase" to SVG::Point which says "the 'point' from SVG itself". Much more informative.