On 20-8-2013 18:53, mathog wrote:
On 20-Aug-2013 08:36, Krzysztof Kosiński wrote:
Ideally the code should look like this:
p0[X] = Inkscape::convert_unit(p0[X], "px", "in");I think this is more expressive that p0[X] * IN_PER_PX.
We disagree on that. I still think the direction of the conversion is ambiguous and that the (still) longer expression is less readable. The older version did not require a maintenance programmer to consult the documentation, the newer one does.
The older version makes it look like the conversion factor is a constant (like it was), instead of a function call (like you propose). I agree with Krzysztof that the new version is nicer. Perhaps one could play with spacing a bit to make it clearer:
p0[X] = Inkscape::convert_unit(p0[X],"px", "in");
But really I don't see much need to look up documentation for this one. To me it seems pretty obvious that the code converts p0 from px to in. I think we should be able to change this code to
p0 = Inkscape::convert_unit(p0,"px", "in");
Overload the conversion function with one supporting Geom::Point, and Geom::Rect maybe? (don't know how often one would want conversion for Rect)
Cheers, Johan