Give access to the underlying number as an lvalue reference

[JohelEGP]

I'm using Dear ImGui.
It has interfaces like ImGui::DragInt:

    IMGUI_API bool          DragInt(const char* label, int* v, float v_speed = 1.0f, int v_min = 0, int v_max = 0, const char* format = "%d", ImGuiSliderFlags flags = 0);  // If v_min >= v_max we have no bound

They are used like this:

ImGui::DragInt("Frames", &frame_frequency.max, 1, frame_frequency.min, 1'000, "%d Hz");

frame_frequency.max is an i32 object.
I'd like that to be a quantity of hertz.
But mp_units doesn't give access to the number of a quantity, so I can't use it.
To enable use of such interfaces, I propose making the underlying number accessible by the name of unsafe_number.
So I propose reverting #412 and renaming number to unsafe_number.
I'm convinced that this is much safer and integrates better with existing code bases:

ImGui::DragInt("Frames", &frame_frequency.max.unsafe_number(), 1, frame_frequency.min.unsafe_number(), 1'000, "%d Hz");

[mpusz]

unsafe_number is a bad name. The ISO C++ Committee will not accept it. Some time ago we made a decision to not use safe or unsafe in our interfaces.

[mpusz]

I can restore an lvalue overload of number() though as it seems to be useful.

[JohelEGP]

I suppose we'll just have to
treat .number() on quantities
like .get() on smart pointers or whatever it was.

[chiphogg]

I'm not a big fan of the interface, because it's not unit-safe: it doesn't force users to name the unit explicitly at the callsite.

Au has a unit-safe version of this: .data_in(unit). It only exists when the provided unit is quantity-equivalent to the quantity's unit.

.number() says, "I don't care what unit it's in, just give me the underlying value!" Is there really a use case for this?

[mpusz]

Well, @JohelEGP just provided one of many use cases for this feature 😉

For cases where we want to be "unit-safe" we have:

mp-units/src/core/include/mp-units/quantity.h

Lines 159 to 164 in c12bf20

	template<Unit U>
	requires requires(quantity q) { q[U{}]; }
	[[nodiscard]] constexpr rep number_in(U) const noexcept
	{
	return (*this)[U{}].number();
	}

This one, however, would not work for @JohelEGP case.

Giving users the possibility to choose which interface is better for them might be a good thing here.

[JohelEGP]

I'm convinced that this is much safer and integrates better with existing code bases:

ImGui::DragInt("Frames", &frame_frequency.max.unsafe_number(), 1, frame_frequency.min.unsafe_number(), 1'000, "%d Hz");

If x.number_in<unit>() returned an lvalue reference to the underlying number
when x is an lvalue reference quantity of unit unit,
then I think this reads safer than the above:

ImGui::DragInt("Frames", &frame_frequency.max.number_in<hertz>(), 1, frame_frequency.min.number_in<hertz>(), 1'000, "%d Hz");

[mpusz]

Having an interface that, for the same single overload depending on some compile-time condition, returns either a reference or a value is a bad idea, in my opinion.

[JohelEGP]

I can see that breaking generic code.

[JohelEGP]

You can implement #412 (comment)'s:

So I think it'll be great to remove all instances of .number(), and replace them with .number_in(units). (You can add something like .stored_number_in(units) if you want a safe handle, similar to Au's .data_in(units).)

IIUC,
.number_in(units) always returns a value,
and .stored_number_in(units) a reference (so units is required to be the quantity object's unit).

[CJCombrink]

Is there really a use case for this?

I recently had to interface my own hand rolled type wrapper (on a microcontroller) and had to call a c function like:

Temperature t;
read_temperature(&t.number());
// read_temperature is in the BSP defined as 
// void read_temperature(float* temp_in_celsius);

Thus such an interface is desired for a units library

[mpusz]

and .stored_number_in(units) a reference (so units is required to be the quantity object's unit).

We can brainstorm how to extend the interface with additional "safer" overload sets. However, I think that the plain number() will stay there anyway. C++ always provides "unsafe" access to the underlying data for many abstractions (including smart pointers). People know how and when to use it.

I am not sure if adding even more "safe" interfaces improves the experience here... For now, I would stay with number() and number_in(U) until we will find a really important use case where none of them work.

[JohelEGP]

That is fair.

Now,
how do we encourage the use of x.number_in<unit>(),
when x.number() is much more succinct.
Users might think the unit of x will never change under a refactor,
or that despite breaking out of the safe abstraction,
that the type unsafe code will remain safe by design.
That seems like asking for trouble.

[chiphogg]

Is there really a use case for this?

I recently had to interface my own hand rolled type wrapper (on a microcontroller) and had to call a c function like:

Temperature t;
read_temperature(&t.number());
// read_temperature is in the BSP defined as 
// void read_temperature(float* temp_in_celsius);

Thus such an interface is desired for a units library

An lvalue interface is surely desirable, but I think the question is whether it should include explicit units at the callsite. Your example above would work just as well with something like the following:

quantity_point<celsius, float> t;
read_temperature(&t.stored_data_in(celsius));

The difference is that it has better callsite readability --- and, as @JohelEGP pointed out, is safer under future refactorings.

Well, @JohelEGP just provided one of many use cases for this feature 😉

Did he? I didn't see the "I don't care about the unit" part. Rather, it seems to me that his use case does care about the unit!

Now, how do we encourage the use of x.number_in<unit>(), when x.number() is much more succinct. Users might think the unit of x will never change under a refactor, or that despite breaking out of the safe abstraction, that the type unsafe code will remain safe by design. That seems like asking for trouble.

This is the critical question: how to encourage the use of safer APIs? The most effective way I know is to make them the only APIs. 😄 This also has the virtuous effect of encouraging the APIs to be as concise and user-friendly as possible, to minimize user complaints.

Indeed, I got several user complaints about my unit-safe interfaces. However, 100% of them occurred in the design proposal phase, before anyone actually used the interfaces. The years of production use haven't yielded a single complaint.

[JohelEGP]

Au has .data_in(unit), which IIUC, returns what I need.

[mpusz]

Sure, I agree. I am just having a hard time with the naming of such two overload sets. Right now, number() returns reference, and number_in(Unit) returns a value which is fine IMHO. I am also fine with refactoring number() to get the unit and require the unit to have the same magnitude (does not have to be exactly the same unit) as the current one. The problem is how to name those two interfaces to be easy to understand. I am not happy with anything that contains "stored" or "data" in the name.

[JohelEGP]

We could open a new issue, "How to rename the number accessor?"