search

dsharlet / array

C++ multidimensional arrays in the spirit of the STL
Apache License 2.0
198 stars 15 forks source link

Adds reinterpret_const with a test. #86

Closed jiawen closed 1 year ago

jiawen commented 1 year ago

Closes: #85.

jiawen commented 1 year ago

@dsharlet How about this? It's a little "too easy to use" but it's also named reinterpret so it seems fine to me.

All the tests pass.

Any remaining gotchas?

dsharlet commented 1 year ago

I think the readability of this version is a bit questionable, if you read something like a = reinterpret(b), it's a bit surprising that this is removing const.

I think if we're going to keep this API, it should be named reinterpret_const despite the fact that it's a new function instead of an overload (which I do like).

I still think the API that is the same as the existing reinterpret except it uses const_cast instead of reinterpret_cast internally makes the most sense. That way, reinterpret, reinterpret_cast, const_cast, and reinterpret_const all have the same basic API and usage (explicitly specified new type).

FWIW I have always thought it was a bit of a wart that const_cast needed the new type (not just constness) even though its redundant, but I think we should mirror that wart in our API.