bernedom / SI

A header only C++ library that provides type safety and user defined literals for physical units
https://si.dominikberner.ch/doc/
MIT License
486 stars 40 forks source link

Unable to define unit symbol for length_t in miles #119

Closed amesgames closed 1 year ago

amesgames commented 1 year ago

Describe the bug Terrific library, by the way. This could be user error, however I am unable to define a unit symbol for miles (mi), even though miles per hour (mph) works.

To Reproduce Steps to reproduce the behavior:

With the following header file, I am unable to produce a SI::to_string() output that has mi units.

#pragma once

#include <SI/length.h>
#include <SI/velocity.h>

#include <ratio>

namespace SI
{

template <typename _type>
using miles_t = SI::length_t<_type, std::ratio<1609344l, 1000l>>;

template <>
struct unit_symbol<'L', std::ratio<1609344l, 1000l>::type>
    : SI::detail::unit_symbol_impl<'m', 'i'> {};

template <typename _type>
using miles_per_hour_t = SI::velocity_t<_type, std::ratio<1397l, 3125l>>;

template <>
struct unit_symbol<'v', std::ratio<1397l, 3125l>::type>
    : SI::detail::unit_symbol_impl<'m', 'p', 'h'> {};

}

Expected behavior

The follow should produce the str value "1.000000 mi". Instead, it produces "1.000000 \000m".

constexpr auto miles = SI::miles_t<double>(1);
auto str = SI::to_string(miles);

However, the following works fine and produces "1.000000 mph".

constexpr auto miles_per_hour = SI::miles_per_hour_t<double>(1);
auto str = SI::to_string(miles_per_hour);

Desktop:

bernedom commented 1 year ago

Sorry, I somehow missed your message here. It might be a genuine bug or limitation in the string conversion. I will look into it

bernedom commented 1 year ago

In your code where you define the unit_symbol your are passing the type of the ratio not the ratio itself. Get rid of the ::type and it should work as expected.

template <>
struct unit_symbol<'L', std::ratio<1609344l, 1000l>>
    : SI::detail::unit_symbol_impl<'m', 'i'> {};

No clue why it works with mph.

I will try to add a warning or error to the template to catch these errors easier but I am not sure if it is possible If you would like to add the imperial units feel free to open a PR ;)

amesgames commented 1 year ago

That did it.

When I get back to this project, I will certainly issue a PR.

Thank you!