Not sure that I have a complere grasp of this web thingy

---------- Forwarded message ----------

From: Ben Fowler <

[hidden email]>

Date: Jul 2, 2005 4:18 PM

Subject: Removing C-Style casts

To:

[hidden email]
In the file libnr/nr-point.h, there are some C-Style casts in the code:

/**

\brief A function to lower the precision of the point

\param places The number of decimal places that should be in

the final number.

This function accomplishes its goal by doing a 10^places and

multipling the X and Y coordinates by that. It then casts that

to an integer (to get rid of all decimal points) and then divides

by 10^places back again.

*/

inline void round (int places = 0) {

_pt[X] = (Coord)(Inkscape::round((double)_pt[X] * pow(10,

places)) / pow(10, places));

_pt[Y] = (Coord)(Inkscape::round((double)_pt[Y] * pow(10,

places)) / pow(10, places));

return;

}

Now casting _pt[] to a double is no-op, and possibly the (Coord) cast is also

unnecessary, or could be converted to a function-style one: Coord( ... ) .

My problem centres on ensuring that the code matches the comment, which

refers to casting to an int. I am fairly sure that the

Inkscape::round( ) function

actually is a wrapper around std::floor( ) which does everything in IEEE double

precision.

IMHO, the Coord type should have a round( ) function, which I have had to

provide under a different name:

namespace NR {

/**

- * A "real" type with sufficient precision for coordinates.

+ * A floating point type with sufficient precision for coordinates.

*

* You may safely assume that double (or even float) provides enough

precision for storing

* on-canvas points, and hence that double provides enough precision

for dot products of

* differences of on-canvas points.

*/

-typedef double Coord;

+ typedef double Coord;

+

+ /**

+ \brief A function to lower the precision of a co-ordinate

+ \param places The number of decimal places that should remain in

+ this co-ordinate.

+

+ This function accomplishes its goal by multipling the co-ordinate by

+ (10^places). It then uses the Inkscape round( ) function (currently

+ implemented with std::floor( )) to remove the surplus precision and

+ then divides by (10^places) to scale back again.

+ */

+ inline Coord set_precision (Coord c, int places = 0) {

+// g_assert( places > 0 ); // Assert on do-nothing

calls and negative number of decimal places

+ double factor = pow(10, places);

+ return Coord((Inkscape::round(c * factor) / factor));;

+ }

} /* namespace NR */

and then I can re-do the function as:

/**

\brief A function to lower the precision of the point

\param places The number of decimal places that should remain in

the co-ordinates of the point.

*/

inline void round (int places = 0) {

// g_assert( places > 0 ); // Assert on do-nothing

calls and negative number of decimal places

_pt[X] = NR::set_precision(_pt[X], places);

_pt[Y] = NR::set_precision(_pt[Y], places);

return;

}

Is this the way to go?

Would these changes be acceptable in any way?

Should Coord be a class rather than a typedef? Should the round( ) function(s)

which are really floor( ) functions be renamed? Should the round functions(s)

which are really set_precision( ) (if any) be renamed? Should any of these

functions be templates e.g. (untested)

template <class T>

inline void round( int places=0 ) {

// round co-ordinates to places decimal places

}

How are modifications to working code meant to be tested?

I would be happy to submit patches for review, or pursue this in any

other way: I

am trying to see whether it is viable to remove C-Style casts as per

http://www.inkscape.org/cgi-bin/wiki.pl?InkscapeJanitors-------------------------------------------------------

SF.Net email is sponsored by: Discover Easy Linux Migration Strategies

from IBM. Find simple to follow Roadmaps, straightforward articles,

informative Webcasts and more! Get everything you need to get up to

speed, fast. <a href="http://ads.osdn.com/?ad_idt77&alloc_id492&op=click">http://ads.osdn.com/?ad_idt77&alloc_id492&op=click

_______________________________________________

Inkscape-devel mailing list

[hidden email]
https://lists.sourceforge.net/lists/listinfo/inkscape-devel