patterncppMinor

Type-safe cartesian co-ordinates

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

typeordinatescartesiansafe

Problem

I've recently been fiddling around with a type safe implementation of cartesian co-ordinates (and a few operations on those co-ordinates). Often it's easy to get units mixed up: is something in metres, or in some other unit?

The idea for this code is to provide a framework to catch all such errors at compile time. This starts with a simple definition of the unit types we want to support:

distance.hpp

#ifndef UNITS_DISTANCE_HPP_
#define UNITS_DISTANCE_HPP_

#include 

enum struct distance
{
    metre, kilometre, mile
};

std::ostream& operator<<(std::ostream& os, distance d)
{
    switch (d) {
    case distance::metre:
        os << " metres";
        break;
    case distance::kilometre:
        os << " kilometres";
        break;
    case distance::mile:
        os << " miles";
        break;
    }
    return os;
}

#endif

We then build up a quantity type that is templated on our distance type. This supports (explicit) conversions to and from quantity types, and a few simple operations:

quantity.hpp

```
#ifndef UNITS_QUANTITY_HPP
#define UNITS_QUANTITY_HPP

#include "distance.hpp"

#include
#include

namespace detail
{

// Defines a binary operation on quantities, mainly used
// to implement operator+ and operator-. Any binary operator
// implementation should forward to this class with the correct
// function (see the definition below).
template
class binary_operation;

}

// Converts a distance in any format to metres, which is the
// "cannonical" distance unit. Everything is convertible to
// and from metres.
template
struct to_metres;

// Performs a conversion between any distances, e.g.
// m -> km, m -> mi, km -> mi, etc.
template
struct convert;

template
class quantity;

// Given 3 distance points (of the same type), will calculate
// the euclidian distance. These points are expected
// to be calculated by (for example) (x2 - x1), (y2 - y1), (z2 - z1).
template
quantity euclid_distance(quantity x, quantity y, quantity z);

template
std:

Solution

After seeing your code the first time, something didn't feel right. I couldn't quite put my fingers on the problem. However, after going over the code a few times, I came up with a few things that can be changed to improve the code.

Using the right nomenclature

I think the nomenclature is wrong.

Metre, kilometer, and mile are better thought of as units of distance instead of being distances. 12 metres is a distance but metre is not.

Your use of distance and quantity is not quite right. Distance is a quantity + a length unit. When we say distance from point A to point B is 5 kilometres, 5 is the quantity and kilometre is the length unit.

Design of the enum and various classes

It occurred to me that your design violates The Open/Closed Principle.

You have the following enum that's the lynchpin of your entire program.

enum struct distance
{
    metre, kilometre, mile
};

You have class templates with the above enum as template parameters. You have specializations that use the enums. You have user defined suffix operators that depend on the enums.

If you want to add inch to that list, you'll have to search for all the places where these units are used, go through every one of those files, and add another function, another value, another clause that deals with the new unit. These are very intrusive changes, and will most likely introduce bugs.

You shouldn't have to modify existing working code to add new type/enum to the code base.

My suggestion

-
Create a base class called length_unit. Create sub-types metre, kilometre, and mile. Make sure length_unit has the necessary virtual member functions that are needed for supporting the high level functionality.

-
Implement metre, kilometre, and mile in separate files. When the need for adding inch comes, it will be a very simple process.

-
Create a class distance that captures the quantity part and the length unit part.

-
Remove quantity altogether.

-
Make cartesian a regular class, not a class template.

P.S. I have left out the code dealing with angles since they can be updated in a manner similar to distance.

Here are the files and their contents:

length_unit.hpp

#ifndef LENGTH_UNIT_HPP_
#define LENGTH_UNIT_HPP_

#include 

struct length_unit
{
   virtual ~length_unit() {}

   virtual length_unit* copy() const = 0;

   virtual double getScaleFactor() const = 0;  // Scale factor to SI unit.

   virtual std::ostream& write(std::ostream& out) const = 0;
};

inline std::ostream& operator<<(std::ostream& out, length_unit const& unit)
{
   return unit.write(out);
}

#endif

distance.hpp

#ifndef DISTANCE_HPP_
#define DISTANCE_HPP_

#include 
#include 

#include "length_unit.hpp"

class distance
{
   public:

      distance(double d, length_unit const& unit)
         : d_(d), unit_ptr_(unit.copy())
      { }

      distance(double d, std::shared_ptr const& unit_ptr)
         : d_(d), unit_ptr_(unit_ptr)
      { }

      distance scale(double by) const
      {
         return distance(d_ * by, unit_ptr_);
      }

      distance convert(length_unit const& unit) const;

      template  distance convert() const
      {
         return this->convert(unit());
      }

      distance& operator+=(distance const& rhs);
      distance operator+(distance const& rhs) const;

      distance& operator-=(distance const& rhs);
      distance operator-(distance const& rhs) const;
      distance operator-(); // Unary - operator.

      friend std::ostream& operator unit_ptr_;
};

#endif

distance.cpp

#include 
#include "distance.hpp"

distance distance::convert(length_unit const& unit) const
{
   // Scale factor from this distance to SI unit
   double scaleToSI1 = unit_ptr_->getScaleFactor();
   double scaleToSI2 = unit.getScaleFactor();

   double f = scaleToSI1/scaleToSI2;

   return distance(this->d_*f, std::shared_ptr(unit.copy()));
}

distance& distance::operator+=(distance const& rhs)
{
   distance copy = rhs.convert(*(this->unit_ptr_));
   d_ += copy.d_;
   return *this;
}

distance distance::operator+(distance const& rhs) const
{
   distance ret(*this);
   ret += rhs;
   return ret;
}

distance& distance::operator-=(distance const& rhs)
{
   distance copy = rhs.convert(*(this->unit_ptr_));
   d_ -= copy.d_;
   return *this;
}

distance distance::operator-(distance const& rhs) const
{
   distance ret(*this);
   ret -= rhs;
   return ret;
}

distance distance::operator-()
{
   return distance(-d_, unit_ptr_);
}

std::ostream& operator<<(std::ostream& out, distance const& dist)
{
   return out << dist.d_ << " " << *(dist.unit_ptr_);
}

distance euclid_distance(distance const& x, distance const& y, distance const& z)
{
   distance y_copy = y.convert(*(x.unit_ptr_));
   distance z_copy = z.convert(*(x.unit_ptr_));

   return distance(std::sqrt(x.d_ * x.d_ + y_copy.d_ * y_copy.d_ + z_copy.d_ * z_copy.d_), x.unit_ptr_);
}

Notice that distance does not know anything about specific types of units. It i

Code Snippets

enum struct distance
{
    metre, kilometre, mile
};

#ifndef LENGTH_UNIT_HPP_
#define LENGTH_UNIT_HPP_

#include <ostream>

struct length_unit
{
   virtual ~length_unit() {}

   virtual length_unit* copy() const = 0;

   virtual double getScaleFactor() const = 0;  // Scale factor to SI unit.

   virtual std::ostream& write(std::ostream& out) const = 0;
};

inline std::ostream& operator<<(std::ostream& out, length_unit const& unit)
{
   return unit.write(out);
}

#endif

#ifndef DISTANCE_HPP_
#define DISTANCE_HPP_

#include <ostream>
#include <memory>

#include "length_unit.hpp"

class distance
{
   public:

      distance(double d, length_unit const& unit)
         : d_(d), unit_ptr_(unit.copy())
      { }

      distance(double d, std::shared_ptr<length_unit> const& unit_ptr)
         : d_(d), unit_ptr_(unit_ptr)
      { }

      distance scale(double by) const
      {
         return distance(d_ * by, unit_ptr_);
      }

      distance convert(length_unit const& unit) const;

      template <typename unit> distance convert() const
      {
         return this->convert(unit());
      }

      distance& operator+=(distance const& rhs);
      distance operator+(distance const& rhs) const;

      distance& operator-=(distance const& rhs);
      distance operator-(distance const& rhs) const;
      distance operator-(); // Unary - operator.

      friend std::ostream& operator<<(std::ostream& out, distance const& dist);

      friend distance euclid_distance(distance const& x, distance const& y, distance const& z);

   private:

      double d_;
      std::shared_ptr<length_unit> unit_ptr_;
};

#endif

#include <cmath>
#include "distance.hpp"

distance distance::convert(length_unit const& unit) const
{
   // Scale factor from this distance to SI unit
   double scaleToSI1 = unit_ptr_->getScaleFactor();
   double scaleToSI2 = unit.getScaleFactor();

   double f = scaleToSI1/scaleToSI2;

   return distance(this->d_*f, std::shared_ptr<length_unit>(unit.copy()));
}

distance& distance::operator+=(distance const& rhs)
{
   distance copy = rhs.convert(*(this->unit_ptr_));
   d_ += copy.d_;
   return *this;
}

distance distance::operator+(distance const& rhs) const
{
   distance ret(*this);
   ret += rhs;
   return ret;
}

distance& distance::operator-=(distance const& rhs)
{
   distance copy = rhs.convert(*(this->unit_ptr_));
   d_ -= copy.d_;
   return *this;
}

distance distance::operator-(distance const& rhs) const
{
   distance ret(*this);
   ret -= rhs;
   return ret;
}

distance distance::operator-()
{
   return distance(-d_, unit_ptr_);
}

std::ostream& operator<<(std::ostream& out, distance const& dist)
{
   return out << dist.d_ << " " << *(dist.unit_ptr_);
}

distance euclid_distance(distance const& x, distance const& y, distance const& z)
{
   distance y_copy = y.convert(*(x.unit_ptr_));
   distance z_copy = z.convert(*(x.unit_ptr_));

   return distance(std::sqrt(x.d_ * x.d_ + y_copy.d_ * y_copy.d_ + z_copy.d_ * z_copy.d_), x.unit_ptr_);
}

#ifndef CARTESIAN_HPP
#define CARTESIAN_HPP

#include <ostream>

#include "distance.hpp"

struct cartesian
{
   private:

      distance x_, y_, z_;

   public:

      cartesian(distance const& x, distance const& y, distance const& z)
         : x_(x),
         y_(y),
         z_(z)
      { }

      distance x() const { return x_; }
      distance y() const { return y_; }
      distance z() const { return z_; }

      cartesian scale(double by) const
      {
         return cartesian(x_.scale(by), y_.scale(by), z_.scale(by));
      }

      template <typename unit>
      cartesian convert() const
      {
         return cartesian(x_.convert<unit>(), y_.convert<unit>(), z_.convert<unit>());
      }

};

extern distance euclid_distance(cartesian const& a, cartesian const& b);

extern cartesian operator+(cartesian const& a, cartesian const& b);

extern cartesian operator-(cartesian const& a, cartesian const& b);

extern std::ostream& operator<<(std::ostream& os, cartesian const& c);

#endif

Context

StackExchange Code Review Q#82480, answer score: 3

Revisions (0)

No revisions yet.