3 ms·

> I insist on this all the time in code reviews. Variables must have units in their names if there's any ambiguity. For example, `int timeout` becomes `int time

> I insist on this all the time in code reviews. Variables must have units in their names if there's any ambiguity. For example, `int timeout` becomes `int timeout_msec`. Same here. I'm still torn when this gets pushed into the type system, but my general rule of thumb in C++ context is: void FooBar(std::chrono::milliseconds timeout); is OK, because that's a function signature and you'll see the type when you're looking at it, but with variables, `timeout` is not OK, as 99% of the time you'll see it used like: auto timeout = gl_timeout; // or GetTimeoutFromSomewhere(). FooBar(timeout); Common use of `auto` in C++ makes it a PITA to trace down exact type when it matters. (Yes, I use IDE or a language-server-enabled editor when working with C++, and no, I don't have time to stop every 5 seconds to hover my mouse over random symbols to reveal their types.)

physicles 2y ago
Right, your type system can quickly become unwieldy if you try to create a new type for every slight semantic difference. I feel like Go strikes a good balance here with the time.Duration type, which I use wherever I can (my _msec example came from C). Go doesn’t allow implicit conversion between types defined with a typedef, so your code ends up being very explicit about what’s going on.
theamk 2y ago
It should not matter though, because std::chrono is not int-convertible - so is it "milliseconds" or "microseconds" or whatever is an minor implementation detail. You cannot compile FooBar(5000), so there is never confusion in C++ like C has. You have to do explicit "FooBar(std::chrono::milliseconds(500))" or "FooBar(500ms)" if you have literals enabled. And this will handle conversion if needed - you can always do FooBar(500ms) and it will work even if actual type in microseconds. Similarly, your "auto" example will only compile if gl_timeout is a compatible type, so you don't have to worry about units at all when all your intervals are using std::chrono.
OskarS 2y ago
One of my favorite features of std::chrono (which can be a pain to use, but this part is pretty sweet) is that you don't have to specify the exact time unit, just a generic duration. So, combined with chrono literals, both of these work just like expected: std::this_thread::sleep_for(10ms); // sleep for 10 milliseconds std::this_thread::sleep_for(1s); // sleep for one second std::this_thread::sleep_for(50); // does not work, unit is required by type system That's such a cool way to do it: instead of forcing you to specify the exact unit in the signature (milliseconds or seconds), you just say that it's a time duration of some kind, and let the user of the API pick the unit. Very neat!
- twic 2y ago
  I do something similar in Java by taking a java.time.Duration in any method dealing with time. We don't have the snazzy literal syntax, but that means users have to write: someMethodDealingWithTime(Duration.ofMillis(10)); someMethodDealingWithTime(Duration.ofSeconds(1)); someMethodDealingWithTime(50); // does not compile Since these often come from config, i also have a method parseDuration which accepts a variety of simple but unambiguous string formats for these, like "10ms", "1s", "2h30m", "1m100us", "0", "inf", etc. So in config we can write: galactus.requestTimeout=30s No need to bake the unit into the name, but also less possibility of error.
  - TeMPOraL 2y ago
    > i also have a method parseDuration which accepts a variety of simple but unambiguous string formats for these, like "10ms", "1s", "2h30m", "1m100us", "0", "inf", etc. I did that too with parsers for configuration files; my rule of thumb is that the unit has to always be visible somewhere anywhere a numeric parameter occurs - in the type, in the name, or in the value. Like e.g.: // in config file: { ..., "timeout": "10 seconds", ... } // in parsing code: auto& ParseTimeout(const std::string&) -> Expected<std::chrono::milliseconds>; // in a hypothetical intermediary if, for some reason, we need to use a standard numeric type: int timeoutMsec = ....; Wrt. string formats, I usually allowed multiple variants for a given time unit, so e.g. all these were valid and equivalent values: "2h", "2 hour", "2 hours". I'm still not convinced it was the best idea, but the Ops team appreciated it. (I didn't allow mixing time units like "2h30m" in your example, as to simplify parsing into single "read double, read rest as string key into a lookup table" pass, but I'll think about allowing it the next time I'm in such situations. Are there any well-known pros/cons to this?)
    - twic 2y ago
      Mixed unit durations are in ISO 8601, so the idea has had at least some scrutiny: https://en.wikipedia.org/wiki/ISO_8601#Durations https://en.wikipedia.org/wiki/ISO_8601#Durations One place i have run into confusion is being able to express a given span of time in multiple ways. 1m30s and 90s are the same length, but are they the same thing? Should we always normalise? If we do, do we normalise upwards or downwards? This hasn't actually been a problem with time, but i do similar handling with dates, and it turns out we often want to preserve the distinction between 1y6m and 18m. But also sometimes don't. Fun times.
codetrotter 2y ago
> Yes, I use IDE or a language-server-enabled editor when working with C++, and no, I don't have time to stop every 5 seconds to hover my mouse over random symbols to reveal their types. JetBrains does a great thing where they show types for a lot of things as labels all the time instead of having to hover over all the things.
- TeMPOraL 2y ago
  Right; the so-called "inlay hints" are also provided by clangd over LSP, so I have them in my Emacs too. Super helpful, but not always there when I need them.

physicles 2y ago

Right, your type system can quickly become unwieldy if you try to create a new type for every slight semantic difference. I feel like Go strikes a good balance here with the time.Duration type, which I use wherever I can (my _msec example came from C). Go doesn’t allow implicit conversion between types defined with a typedef, so your code ends up being very explicit about what’s going on.

theamk 2y ago

It should not matter though, because std::chrono is not int-convertible - so is it "milliseconds" or "microseconds" or whatever is an minor implementation detail. You cannot compile FooBar(5000), so there is never confusion in C++ like C has. You have to do explicit "FooBar(std::chrono::milliseconds(500))" or "FooBar(500ms)" if you have literals enabled. And this will handle conversion if needed - you can always do FooBar(500ms) and it will work even if actual type in microseconds. Similarly, your "auto" example will only compile if gl_timeout is a compatible type, so you don't have to worry about units at all when all your intervals are using std::chrono.

OskarS 2y ago

One of my favorite features of std::chrono (which can be a pain to use, but this part is pretty sweet) is that you don't have to specify the exact time unit, just a generic duration. So, combined with chrono literals, both of these work just like expected: std::this_thread::sleep_for(10ms); // sleep for 10 milliseconds std::this_thread::sleep_for(1s); // sleep for one second std::this_thread::sleep_for(50); // does not work, unit is required by type system That's such a cool way to do it: instead of forcing you to specify the exact unit in the signature (milliseconds or seconds), you just say that it's a time duration of some kind, and let the user of the API pick the unit. Very neat!

twic 2y ago
I do something similar in Java by taking a java.time.Duration in any method dealing with time. We don't have the snazzy literal syntax, but that means users have to write: someMethodDealingWithTime(Duration.ofMillis(10)); someMethodDealingWithTime(Duration.ofSeconds(1)); someMethodDealingWithTime(50); // does not compile Since these often come from config, i also have a method parseDuration which accepts a variety of simple but unambiguous string formats for these, like "10ms", "1s", "2h30m", "1m100us", "0", "inf", etc. So in config we can write: galactus.requestTimeout=30s No need to bake the unit into the name, but also less possibility of error.
- TeMPOraL 2y ago
  > i also have a method parseDuration which accepts a variety of simple but unambiguous string formats for these, like "10ms", "1s", "2h30m", "1m100us", "0", "inf", etc. I did that too with parsers for configuration files; my rule of thumb is that the unit has to always be visible somewhere anywhere a numeric parameter occurs - in the type, in the name, or in the value. Like e.g.: // in config file: { ..., "timeout": "10 seconds", ... } // in parsing code: auto& ParseTimeout(const std::string&) -> Expected<std::chrono::milliseconds>; // in a hypothetical intermediary if, for some reason, we need to use a standard numeric type: int timeoutMsec = ....; Wrt. string formats, I usually allowed multiple variants for a given time unit, so e.g. all these were valid and equivalent values: "2h", "2 hour", "2 hours". I'm still not convinced it was the best idea, but the Ops team appreciated it. (I didn't allow mixing time units like "2h30m" in your example, as to simplify parsing into single "read double, read rest as string key into a lookup table" pass, but I'll think about allowing it the next time I'm in such situations. Are there any well-known pros/cons to this?)
  - twic 2y ago
    Mixed unit durations are in ISO 8601, so the idea has had at least some scrutiny: https://en.wikipedia.org/wiki/ISO_8601#Durations https://en.wikipedia.org/wiki/ISO_8601#Durations One place i have run into confusion is being able to express a given span of time in multiple ways. 1m30s and 90s are the same length, but are they the same thing? Should we always normalise? If we do, do we normalise upwards or downwards? This hasn't actually been a problem with time, but i do similar handling with dates, and it turns out we often want to preserve the distinction between 1y6m and 18m. But also sometimes don't. Fun times.

codetrotter 2y ago

> Yes, I use IDE or a language-server-enabled editor when working with C++, and no, I don't have time to stop every 5 seconds to hover my mouse over random symbols to reveal their types. JetBrains does a great thing where they show types for a lot of things as labels all the time instead of having to hover over all the things.

TeMPOraL 2y ago
Right; the so-called "inlay hints" are also provided by clangd over LSP, so I have them in my Emacs too. Super helpful, but not always there when I need them.