4 ms·
request.setConnectTimeout(getDefaultTimeoutFromEnvironment().seconds) This is actually a very good example of what not to do, with the mistake being in whoever
by 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 agoThis 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 agoI think you should rather do .setPrice(priceInCents=100, massInKg=2, timeoutInMs=300)
- gregmac 5y agoI'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 agoValues 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))
- LgWoodenBadger 5y agoThat's one of the benefits of having it exposed as a type-enforced parameter. setConnectTimeout() could take a Duration, which contains the amount and the unit, and therefore wouldn't care if consumer A provided a timeout in seconds, and consumer B provided a timeout in milliseconds.
- gregmac 5y agoTotally agree, but then I would expect the code would be: request.setConnectTimeout(getDefaultTimeoutFromEnvironment()) with getDefaultTimeoutFromEnvironment() returning a Duration. Ideally this is consistent throughout the codebase, so that anything that uses a primitive type for time is explicitly labelled, and anything using a Duration can just be called "Timeout" or whatever.
- contravariant 5y agoAll fair points. In this example I was suggesting what this might look like at the border of the application where you need to talk to some (standard) library which doesn't use the same convention.