Skip to main content
deleted 266 characters in body
Source Link
Michael Urman
  • 3.9k
  • 1
  • 12
  • 23

I agree that more context is necessary to know whether this is the right approach. When I've worked with ray tracing, you more often needed to know the closest item, or that a distance was above or below some threshold, but did not need to know what the actual distance was. In those contexts, at the time, I worked with raw numbers that were explicitly the square of the distance, returned from a function called distance2; I think I prefer that approach.

It looks like you're trying to hide the squares and square roots. Abstractions like that can be very useful, but come at a cost. Without the abstraction, the programmer has to track the complexity, and this means fewer other things will fit in the programmer's head at the same time. When hiding this optimization, however, you either have to make another time vs. space trade-off (for instance, you initially save time by delaying or avoiding the sqrt call, but then have to choose whether to cache the results; caching saves time but costs space). Essentially the class is trying to guess what the programmer needs, and this often goes badly.

If memory use is a concern, you can mitigate the extra space requirements of a cache by adding complexity. Relying on the fact that no distances are negative; using a negative distance can mark whether it is the distance or its square. But despite saving the extra storage, that complexity shows up in all operators.

How far should this abstraction go? Should you have specified comparison operators such as operator<? If so, should you specify them both for two DistanceExpression<T> values as well as for a T and a DistanceExpression<T> value? This could be useful if you want to write if (distance(p, q) < 10) without requiring the sqrt call. But again if (distance2(p, q) < 100) is also quite clear.

Reviewing beyond your explicit question, is it obvious that operator*(Point<T, N>, Point<T, N>) is a dot product and not a cross-product for N=3? Does that even make sense on a class named Point instead of Vector? Consider carefully when you want to name methods instead of relying on operators.

Goodgood call on marking DistanceExpression<T>'s constructor explicit, although was surprised that you initialized its _data member using parenthesis () instead of curly braces {}. Old habits die hard! I also prefer avoiding the leading underscore on member names, but it appears that only when the next letter is capital is it categorically reserved. Beware thin ice.

I agree that more context is necessary to know whether this is the right approach. When I've worked with ray tracing, you more often needed to know the closest item, or that a distance was above or below some threshold, but did not need to know what the actual distance was. In those contexts, at the time, I worked with raw numbers that were explicitly the square of the distance, returned from a function called distance2; I think I prefer that approach.

It looks like you're trying to hide the squares and square roots. Abstractions like that can be very useful, but come at a cost. Without the abstraction, the programmer has to track the complexity, and this means fewer other things will fit in the programmer's head at the same time. When hiding this optimization, however, you either have to make another time vs. space trade-off (for instance, you initially save time by delaying or avoiding the sqrt call, but then have to choose whether to cache the results; caching saves time but costs space). Essentially the class is trying to guess what the programmer needs, and this often goes badly.

If memory use is a concern, you can mitigate the extra space requirements of a cache by adding complexity. Relying on the fact that no distances are negative; using a negative distance can mark whether it is the distance or its square. But despite saving the extra storage, that complexity shows up in all operators.

How far should this abstraction go? Should you have specified comparison operators such as operator<? If so, should you specify them both for two DistanceExpression<T> values as well as for a T and a DistanceExpression<T> value? This could be useful if you want to write if (distance(p, q) < 10) without requiring the sqrt call. But again if (distance2(p, q) < 100) is also quite clear.

Reviewing beyond your explicit question, is it obvious that operator*(Point<T, N>, Point<T, N>) is a dot product and not a cross-product for N=3? Does that even make sense on a class named Point instead of Vector? Consider carefully when you want to name methods instead of relying on operators.

Good call on marking DistanceExpression<T>'s constructor explicit, although was surprised that you initialized its _data member using parenthesis () instead of curly braces {}. Old habits die hard! I also prefer avoiding the leading underscore on member names, but it appears that only when the next letter is capital is it categorically reserved. Beware thin ice.

I agree that more context is necessary to know whether this is the right approach. When I've worked with ray tracing, you more often needed to know the closest item, or that a distance was above or below some threshold, but did not need to know what the actual distance was. In those contexts, at the time, I worked with raw numbers that were explicitly the square of the distance, returned from a function called distance2; I think I prefer that approach.

It looks like you're trying to hide the squares and square roots. Abstractions like that can be very useful, but come at a cost. Without the abstraction, the programmer has to track the complexity, and this means fewer other things will fit in the programmer's head at the same time. When hiding this optimization, however, you either have to make another time vs. space trade-off (for instance, you initially save time by delaying or avoiding the sqrt call, but then have to choose whether to cache the results; caching saves time but costs space). Essentially the class is trying to guess what the programmer needs, and this often goes badly.

If memory use is a concern, you can mitigate the extra space requirements of a cache by adding complexity. Relying on the fact that no distances are negative; using a negative distance can mark whether it is the distance or its square. But despite saving the extra storage, that complexity shows up in all operators.

How far should this abstraction go? Should you have specified comparison operators such as operator<? If so, should you specify them both for two DistanceExpression<T> values as well as for a T and a DistanceExpression<T> value? This could be useful if you want to write if (distance(p, q) < 10) without requiring the sqrt call. But again if (distance2(p, q) < 100) is also quite clear.

Reviewing beyond your explicit question, good call on marking DistanceExpression<T>'s constructor explicit, although was surprised that you initialized its _data member using parenthesis () instead of curly braces {}. Old habits die hard! I also prefer avoiding the leading underscore on member names, but it appears that only when the next letter is capital is it categorically reserved. Beware thin ice.

Source Link
Michael Urman
  • 3.9k
  • 1
  • 12
  • 23

I agree that more context is necessary to know whether this is the right approach. When I've worked with ray tracing, you more often needed to know the closest item, or that a distance was above or below some threshold, but did not need to know what the actual distance was. In those contexts, at the time, I worked with raw numbers that were explicitly the square of the distance, returned from a function called distance2; I think I prefer that approach.

It looks like you're trying to hide the squares and square roots. Abstractions like that can be very useful, but come at a cost. Without the abstraction, the programmer has to track the complexity, and this means fewer other things will fit in the programmer's head at the same time. When hiding this optimization, however, you either have to make another time vs. space trade-off (for instance, you initially save time by delaying or avoiding the sqrt call, but then have to choose whether to cache the results; caching saves time but costs space). Essentially the class is trying to guess what the programmer needs, and this often goes badly.

If memory use is a concern, you can mitigate the extra space requirements of a cache by adding complexity. Relying on the fact that no distances are negative; using a negative distance can mark whether it is the distance or its square. But despite saving the extra storage, that complexity shows up in all operators.

How far should this abstraction go? Should you have specified comparison operators such as operator<? If so, should you specify them both for two DistanceExpression<T> values as well as for a T and a DistanceExpression<T> value? This could be useful if you want to write if (distance(p, q) < 10) without requiring the sqrt call. But again if (distance2(p, q) < 100) is also quite clear.

Reviewing beyond your explicit question, is it obvious that operator*(Point<T, N>, Point<T, N>) is a dot product and not a cross-product for N=3? Does that even make sense on a class named Point instead of Vector? Consider carefully when you want to name methods instead of relying on operators.

Good call on marking DistanceExpression<T>'s constructor explicit, although was surprised that you initialized its _data member using parenthesis () instead of curly braces {}. Old habits die hard! I also prefer avoiding the leading underscore on member names, but it appears that only when the next letter is capital is it categorically reserved. Beware thin ice.