4 ms·

> The only people I've ever met that think this is unnecessary are also the same people that ship a lot of bugs for other people to fix. I find feelings about

> The only people I've ever met that think this is unnecessary are also the same people that ship a lot of bugs for other people to fix. I find feelings about this depend a lot on how much ceremony the type system involves; and just how many units and references to them there are in the system. Asking bash coders to write "sleep 5s" instead of "sleep 5" - I doubt you'd get any objections at all. But if you're putting a foo.bar.Duration on a getDefaultTimeoutFromEnvironment on a setConnectTimeout on a HttpRequest on a HttpRequestInitializer on a NetHttpTransport on a ProviderCredential to make a simple get request? People who've come from less ceremony-heavy languages might feel less productive, despite producing 10x the lines of code.

contravariant 5y ago
Well ignoring the silly stuff this would just boil down to something like: request = NetHttpTransport.Request() request.setConnectTimeout(getDefaultTimeoutFromEnvironment().seconds) which is verbose, but at least it's fairly clear. And yes I'm calling a class named HttpRequestInitializer silly, I don't care if some language decided it should exist.
- sethammons 5y ago
  This is where I divert. You just hard coded seconds into your test. Now your tests that cover this must take seconds to finish; thankfully you were not testing hours or days! My last shop was a Go shop and one test I think shows this off was an SMTP server and we needed to test the timeouts at different states. The test spun up half a dozen instances of the server, got each into the right smtp state and verified timeouts in under 10ms. The environment that set the timeout would either be "timeout_ms=500" or "timeout=500ms" (or whatever). This is where you handle that :)
  - catlifeonmars 5y ago
    This is actually revealing a different problem: the system clock as an implicit dependency. YMMV depending on support of underlying libraries, but I will typically make the clock an explicit dependency of a struct/function. In Go, usually it’s a struct field `now func() time.Time` with a default value of `time.Now`.
  - dahfizz 5y ago
    Many timeout functions take seconds as a floating point. So you could time out on 0.05 seconds (5 milis). But now the code is clear and less prone to bugs.
    - sethammons 5y ago
      In Go, you don't pass a float, you pass a duration
      - catlifeonmars 5y ago
        Which is unitless, hence there is no problem. `time.Duration.Seconds()` returns a floating point.
  - contravariant 5y ago
    Not sure I fully understood your objection, but the reason I specified seconds is because I presumed the setConnectTimeout to be part of the default HTTP library, which likely doesn't adhere to the same conventions, and that it expected seconds (which seem to be the usual for http libraries as far as I can tell). Of course if the setConnectTimeout method was part of the same application you could just pass the timeout directly, but at the boundary of your application you're still going to have to specify at some point which unit you want.
  - dnadler 5y ago
    The test could simply mock the default value to something reasonable like '.1 seconds' and test that duration instead, so I don't think this is a real problem.
  - hansvm 5y ago
    If you're testing things with timeouts it's often a good practice to mock out the system clock anyway. That allows testing your timeouts to be nearly instantaneous and also catches edge cases like the clock not moving forward, having a low resolution, moving backward, ... deterministically.
- michaelt 5y ago
  A class named 'HttpRequestInitializer' and taking 10 lines to set a timeout on a HTTP request isn't merely hypothetical: https://developers.google.com/api-client-library/java/google-api-java-client/errors#timeouts https://developers.google.com/api-client-library/java/google... - and that's not counting any import statements. (although getDefaultTimeoutFromEnvironment was artistic license on my part)
  - contravariant 5y ago
    True, but like I said that doesn't make it not silly. Just harder to fix. Edit: Also, overriding a class method dynamically inside a function? I usually program python these days and even I think that's wild.
    - _ea1k 5y ago
      Its an interface with only method. I suspect that most of us would just us a lambda now.
      - contravariant 5y ago
        I'll never quite understand why it wasn't simply a function in the first place.
        dtech 5y ago
        Java pre-8 only had anonymous classes, there were no lambdas
        fiddlerwoaroof 5y ago
        And Java lambdas are still syntax sugar for one-method anonymous classes.
- gregmac 5y ago
  request.setConnectTimeout(getDefaultTimeoutFromEnvironment().seconds) This is actually a very good example of what not to do, with the mistake being in whoever implemented setConnectTimeout() I actually don't know this particular API, but I'm used to timeouts being in milliseconds, so that code looks wrong to me. Much better API, and what the article is talking about, is to change this to: .setConnectTimeoutMilliseconds(getDefaultTimeoutFromEnvironment().seconds) Now the mistake is obvious, and even if the original developer doesn't notice it will stick out in a PR or even a causal glance.
  - makapuf 5y ago
    This example is nice but if you put arguments metadata in the function name, you have to have one main argument, the function name can prove cumbersome if you have 3 or 4 arguments with units like .setPricePerMassInCentsPerKilogramsWithTimeoutInMilliSeconds(100,2,300)
    - elcomet 5y ago
      I think you should rather do .setPrice(priceInCents=100, massInKg=2, timeoutInMs=300)
    - gregmac 5y ago
      I'd argue there are better API patterns for this though -- keeping in mind this values code readability (and correctness) over micro-optimization: .setPriceInCents(100); .setMassInKilograms(2); .setTimeoutInMilliseconds(300); or .calculate({ priceInCents = 100, massInKilograms: 2, timeoutInMilliseconds: 300, });
      - HelloNurse 5y ago
        Values with an intrinsic unit scale up to many units and values. If you declare the parameters of a "calculate" function as USACurrency, Weight and Duration you can write calculate(100cent, 2Kg, 300s) calculate(0.1dollar,4.7lb/*approximate*/,5min) calculate(something.price(),whatever.weight(),options.getDuration("exampleTimeout")) calculate(USD(0.1),Kg(2),Minute(5))
vvillena 5y ago
The 80/20 approach of renaming "timeout" to "timeoutSeconds" or "timeoutMillis" is also valid. They key takeaway is to not make assumptions.
rowanajmarshall 5y ago
> I find feelings about this depend a lot on how much ceremony the type system involves; and just how many units and references to them there are in the system. Ideally it'd be as simple as: typealias Dollars = BigDecimal typealias Cents = Long that's valid Kotlin, but the equivalent is doable in most languages nowadays (Java being a big exception).
- dtech 5y ago
  I recommend against that, and use proper (wrapper) types. I don't know Kotlin, but in most languages if you alias 2 types to the same base type, for example Seconds and Minutes to Long, the compiler will happily allow you to mix all 3 of them, defeating the protection full types would bring.
  - stefs 5y ago
    that's correct. typealiases are the wrong solution here. the better solution would be value classes, but of course, the unit shouldn't be the type.
- dragonwriter 5y ago
  > typealias Dollars = BigDecimal > typealias Cents = Long If I ever have to deal with monetary values in a program where someone thought this was a good idea, ... well, it really won't be the worst thing I’ve ever dealt with, but, still. (If you have dollars and cents as types in the same program, they darn well better work properly together, which is unlikely to work if they are both just aliases for basic numeric types.)
- stefs 5y ago
  don't do this! typealias Cents = Long typealias Meters = Long Cents(3) + Meters(4) = 7L that's exactly the thing we want to prevent. the classes shouldn't be for units at all, the type should represent the type of unit. so instead of a class Seconds you should have a class Duration, instead of class Meters you should have a type Distance. that's because the unit of the different types can be converted between each other. Dollars and Cents are a bit of a bad example because currencies can't be easily converted between each other, as conversion is dependent on many factors that change over time. meters, yards, feet, lightyears, miles, chain, furlongs, whatever describe a multiple of the same thing though, so a different type for each unit isn't necessary, as the input that was used to create the instance isn' usually needed. the counter example would be datetime with timezones - a datetime with a timezone convers a lot more information than the datetime converted to UTC.
deleted 5y ago
[deleted]

contravariant 5y ago

Well ignoring the silly stuff this would just boil down to something like: request = NetHttpTransport.Request() request.setConnectTimeout(getDefaultTimeoutFromEnvironment().seconds) which is verbose, but at least it's fairly clear. And yes I'm calling a class named HttpRequestInitializer silly, I don't care if some language decided it should exist.

sethammons 5y ago
This is where I divert. You just hard coded seconds into your test. Now your tests that cover this must take seconds to finish; thankfully you were not testing hours or days! My last shop was a Go shop and one test I think shows this off was an SMTP server and we needed to test the timeouts at different states. The test spun up half a dozen instances of the server, got each into the right smtp state and verified timeouts in under 10ms. The environment that set the timeout would either be "timeout_ms=500" or "timeout=500ms" (or whatever). This is where you handle that :)
- catlifeonmars 5y ago
  This is actually revealing a different problem: the system clock as an implicit dependency. YMMV depending on support of underlying libraries, but I will typically make the clock an explicit dependency of a struct/function. In Go, usually it’s a struct field `now func() time.Time` with a default value of `time.Now`.
- dahfizz 5y ago
  Many timeout functions take seconds as a floating point. So you could time out on 0.05 seconds (5 milis). But now the code is clear and less prone to bugs.
  - sethammons 5y ago
    In Go, you don't pass a float, you pass a duration
    - catlifeonmars 5y ago
      Which is unitless, hence there is no problem. `time.Duration.Seconds()` returns a floating point.
- contravariant 5y ago
  Not sure I fully understood your objection, but the reason I specified seconds is because I presumed the setConnectTimeout to be part of the default HTTP library, which likely doesn't adhere to the same conventions, and that it expected seconds (which seem to be the usual for http libraries as far as I can tell). Of course if the setConnectTimeout method was part of the same application you could just pass the timeout directly, but at the boundary of your application you're still going to have to specify at some point which unit you want.
- dnadler 5y ago
  The test could simply mock the default value to something reasonable like '.1 seconds' and test that duration instead, so I don't think this is a real problem.
- hansvm 5y ago
  If you're testing things with timeouts it's often a good practice to mock out the system clock anyway. That allows testing your timeouts to be nearly instantaneous and also catches edge cases like the clock not moving forward, having a low resolution, moving backward, ... deterministically.
michaelt 5y ago
A class named 'HttpRequestInitializer' and taking 10 lines to set a timeout on a HTTP request isn't merely hypothetical: https://developers.google.com/api-client-library/java/google-api-java-client/errors#timeouts https://developers.google.com/api-client-library/java/google... - and that's not counting any import statements. (although getDefaultTimeoutFromEnvironment was artistic license on my part)
- contravariant 5y ago
  True, but like I said that doesn't make it not silly. Just harder to fix. Edit: Also, overriding a class method dynamically inside a function? I usually program python these days and even I think that's wild.
  - _ea1k 5y ago
    Its an interface with only method. I suspect that most of us would just us a lambda now.
    - contravariant 5y ago
      I'll never quite understand why it wasn't simply a function in the first place.
      - dtech 5y ago
        Java pre-8 only had anonymous classes, there were no lambdas
        fiddlerwoaroof 5y ago
        And Java lambdas are still syntax sugar for one-method anonymous classes.
gregmac 5y ago
request.setConnectTimeout(getDefaultTimeoutFromEnvironment().seconds) This is actually a very good example of what not to do, with the mistake being in whoever implemented setConnectTimeout() I actually don't know this particular API, but I'm used to timeouts being in milliseconds, so that code looks wrong to me. Much better API, and what the article is talking about, is to change this to: .setConnectTimeoutMilliseconds(getDefaultTimeoutFromEnvironment().seconds) Now the mistake is obvious, and even if the original developer doesn't notice it will stick out in a PR or even a causal glance.
- makapuf 5y ago
  This example is nice but if you put arguments metadata in the function name, you have to have one main argument, the function name can prove cumbersome if you have 3 or 4 arguments with units like .setPricePerMassInCentsPerKilogramsWithTimeoutInMilliSeconds(100,2,300)
  - elcomet 5y ago
    I think you should rather do .setPrice(priceInCents=100, massInKg=2, timeoutInMs=300)
  - gregmac 5y ago
    I'd argue there are better API patterns for this though -- keeping in mind this values code readability (and correctness) over micro-optimization: .setPriceInCents(100); .setMassInKilograms(2); .setTimeoutInMilliseconds(300); or .calculate({ priceInCents = 100, massInKilograms: 2, timeoutInMilliseconds: 300, });
    - HelloNurse 5y ago
      Values with an intrinsic unit scale up to many units and values. If you declare the parameters of a "calculate" function as USACurrency, Weight and Duration you can write calculate(100cent, 2Kg, 300s) calculate(0.1dollar,4.7lb/*approximate*/,5min) calculate(something.price(),whatever.weight(),options.getDuration("exampleTimeout")) calculate(USD(0.1),Kg(2),Minute(5))

vvillena 5y ago

The 80/20 approach of renaming "timeout" to "timeoutSeconds" or "timeoutMillis" is also valid. They key takeaway is to not make assumptions.

rowanajmarshall 5y ago

> I find feelings about this depend a lot on how much ceremony the type system involves; and just how many units and references to them there are in the system. Ideally it'd be as simple as: typealias Dollars = BigDecimal typealias Cents = Long that's valid Kotlin, but the equivalent is doable in most languages nowadays (Java being a big exception).

dtech 5y ago
I recommend against that, and use proper (wrapper) types. I don't know Kotlin, but in most languages if you alias 2 types to the same base type, for example Seconds and Minutes to Long, the compiler will happily allow you to mix all 3 of them, defeating the protection full types would bring.
- stefs 5y ago
  that's correct. typealiases are the wrong solution here. the better solution would be value classes, but of course, the unit shouldn't be the type.
dragonwriter 5y ago
> typealias Dollars = BigDecimal > typealias Cents = Long If I ever have to deal with monetary values in a program where someone thought this was a good idea, ... well, it really won't be the worst thing I’ve ever dealt with, but, still. (If you have dollars and cents as types in the same program, they darn well better work properly together, which is unlikely to work if they are both just aliases for basic numeric types.)
stefs 5y ago
don't do this! typealias Cents = Long typealias Meters = Long Cents(3) + Meters(4) = 7L that's exactly the thing we want to prevent. the classes shouldn't be for units at all, the type should represent the type of unit. so instead of a class Seconds you should have a class Duration, instead of class Meters you should have a type Distance. that's because the unit of the different types can be converted between each other. Dollars and Cents are a bit of a bad example because currencies can't be easily converted between each other, as conversion is dependent on many factors that change over time. meters, yards, feet, lightyears, miles, chain, furlongs, whatever describe a multiple of the same thing though, so a different type for each unit isn't necessary, as the input that was used to create the instance isn' usually needed. the counter example would be datetime with timezones - a datetime with a timezone convers a lot more information than the datetime converted to UTC.

deleted 5y ago

[deleted]