3

I have written a class to represent bearings (angles with a nautical theme, and a specific normalisation range). In the program, it is necessary to perform some mathematical operations on them, so I've overloaded the +, -, * and / operators (this is in C++).

My question is: what operators are mathematically well defined for such a class? Or more specifically, in the following code, are there any operators defined that I shouldn't have defined, and are there any operators undefined that I should have defined?

constexpr inline Bearing operator+(Bearing lhs, Bearing rhs) noexcept
{
    return Bearing::deg(lhs.getInDegrees() + rhs.getInDegrees());
}

constexpr inline Bearing operator-(Bearing lhs, Bearing rhs) noexcept
{
    return Bearing::deg(lhs.getInDegrees() - rhs.getInDegrees());
}

template <
    typename T,
    typename = EnableIfNumeric<T>
> constexpr inline Bearing operator*(Bearing lhs, T rhs) noexcept
{
    return Bearing::deg(lhs.getInDegrees() * static_cast<Bearing::ValueType>(rhs));
}

template <
    typename T,
    typename = EnableIfNumeric<T>
> constexpr inline Bearing operator*(T lhs, Bearing rhs) noexcept
{
    return Bearing::deg(static_cast<Bearing::ValueType>(lhs) * rhs.getInDegrees());
}


template <
    typename T,
    typename = EnableIfNumeric<T>
> constexpr inline Bearing operator/(Bearing lhs, T rhs) noexcept
{
    return Bearing::deg(lhs.getInDegrees() / static_cast<Bearing::ValueType>(rhs));
}

template <
    typename T,
    typename = EnableIfNumeric<T>
> constexpr inline Bearing operator/(T lhs, Bearing rhs) noexcept; // Intentionally not defined

constexpr inline Bearing::ValueType operator/(Bearing lhs, Bearing rhs) noexcept
{
    return lhs.getInDegrees() / rhs.getInDegrees();
}

// Bearing has value semantics
constexpr inline Bearing operator+=(Bearing lhs, Bearing rhs) noexcept; // Intentionally not defined
constexpr inline Bearing operator-=(Bearing lhs, Bearing rhs) noexcept; // Intentionally not defined
constexpr inline Bearing operator*=(Bearing lhs, Bearing rhs) noexcept; // Intentionally not defined
constexpr inline Bearing operator/=(Bearing lhs, Bearing rhs) noexcept; // Intentionally not defined


constexpr inline bool operator==(Bearing lhs, Bearing rhs) noexcept
{
    return lhs.getInDegrees() == rhs.getInDegrees();
}

constexpr inline bool operator!=(Bearing lhs, Bearing rhs) noexcept
{
    return !(lhs == rhs);
}

In words:

  • (not shown in above code) Bearing instances must be created from static methods Bearing::deg or Bearing::rad, so the units are explicit at initialisation
  • Bearings may be added to or subtracted from other Bearings only
  • Bearings may be multiplied only by numeric types (not other Bearings)
  • Bearings may be divided only numeric types
  • Numeric types may not be divided by Bearings
  • Bearings may be divided by other Bearings, yielding a floating point value
  • Bearings have equality comparison operators, but no inequality comparison operators (because angles wrap around, both a < b and b < a are true if a and b are Bearings)
  • (note that there are member functions to determine the absolute and clockwise/anticlockwise distances between one bearing and another, so whatever you might want to do with inequality comparison operators should be possible)

Note that, by "normalisation", I mean wrapping the angles around so that they are always in the range [0, 360), or [-180, 180), and that this operation is only performed at the client's request, not after every operation.

P.S. I think this question is a good fit for Programmers, but if several people think it is a better fit for Code Review, then I will consider moving it there.

  • Many of your operations only make sense for relative values but not for absolute values. – CodesInChaos Feb 4 '15 at 16:41
  • @CodesInChaos I would love to know the theoretical basis for your statement, if you could provide it in an answer – wakjah Feb 4 '15 at 17:22
  • 1
    Because "2 * east = south" makes no sense. – CodesInChaos Feb 4 '15 at 21:02
3

This depends on what "mathematically well defined" means. All of your functions are well defined in the sense of having a unique definition. However, multiplication and division are problematic, since they are not guaranteeing

(b * n) * m == b * (n * m)

nor

(b * n) / m == b * (n / m)

where b is a bearing and n is a numeric value, and that is what you might expect. For example, if b = 45° and n = m = 8, you get

(b * n) / m == 360° / 8 == 0° / 8 == 0°

but b * (n / m) == 45° * 1 == 45°

(or with multiplication, set m = 1/8, which shows you essentially the same).

So if you want to make this really foolproof, I suggest you add a conversion operator from a Bearing to a float by an interval given by the caller (for example, 0° to 360°or with multiplication, or set -180° to 180°). Multiplication and division should be done only by converting a bearing to a number using this range, then applying the operation, and afterwards converting back to a bearing. That would eliminate every disambiguties.

  • I probably should have mentioned in the question that normalisation (wrapping around from 360 -> 0) is performed only at the client's request, not after every operation. Does this change matters? (it does for your specific example, but are there any counter examples) – wakjah Feb 4 '15 at 13:59
  • @wakjah: as long as you don't normalise, you are working with ordinary floats. But if there is no automatic normalisation after every operation, the outcome of a sequence of operations will depend of if and when one applies normalisation. Sounds unintuitive and pretty error prone to me. – Doc Brown Feb 4 '15 at 14:08
  • I see your point... So are you saying, in other words, that the best thing would be to: 1. have automatic normalisation, and 2. provide no operator to divide by numerics? – wakjah Feb 4 '15 at 14:17
  • I implemented the two suggestions I inferred from your answer and the resulting class is much better than before. Thanks! – wakjah Feb 4 '15 at 16:14
  • @wakjah: at least you should have automatic normalisation before each equality check and each ToString() operation. But I would probably normalise after each operation, since this will keep the rounding errors of limited float precision more predictable. And do you really need division? The fact it will behave unusual does not mean it will be useless. So decide for yourself what is more important for you: to have a division operator with some restrictions, or to make your component fool-proof. – Doc Brown Feb 4 '15 at 16:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.