-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add duration and byte-count formatters #188
base: main
Are you sure you want to change the base?
Conversation
Click here for code coverage report: https://jenkins.flo.pub/job/flowcommerce/job/lib-util/job/PR-188/4/scoverage-report/ |
Click here for code coverage report: https://jenkins.flo.pub/job/flowcommerce/job/lib-util/job/PR-188/5/scoverage-report/ |
|
||
object ByteFormatter { | ||
def byteCountSI(bytes: Long): String = { | ||
format(bytes, true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how you feel about flag arguments but throwing in an alternative, e.g., format(value, multiplier)
where the multiplier has only two cases (SI, Binary) and the list of symbols is derived from it with pattern matching.
|
||
object DurationFormatter { | ||
import ByteFormatter.{df0, df1, df2} | ||
private val units = Seq((1000L, "ns"), (1000L, "us"), (1000L, "ms"), (60L, "sec"), (60L, "min"), (24L, "h")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not μs
and s
? I'd suggest m
instead of min
, because while m
is very common for durations, although techinically that's a meter, and d
for days (also avois "days" vs "day") 🤷♂️ FWIW, Go (hence k8s and all infra stuff written in Go) formats durations as, e.g., 2d9h45m19s
.
} else { | ||
val signum = if (bytes < 0) "-" else "" | ||
val exponent = getExponent(absBytes, baseValue) | ||
val divisor = Math.pow(baseValue.toDouble, exponent.toDouble) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. Seems both simpler and more precise to just return both exponent and divisor from getExponent. (In the recursive step, exponent
becomes exponent+1
and divisor
becomes divisor*baseValue
.) Or just look it up like the unit perhaps.
"TiB values" in { | ||
ByteFormatter.byteCountBinary(1024L * 1024 * 1024 * 1024) mustBe "1 TiB" | ||
ByteFormatter.byteCountBinary(1024L * 1024 * 1024 * 1024 * 1024 - 1) mustBe "1023 TiB" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about larger values; can the unit lookup fail?
What's the context? |
Click here for code coverage report: https://jenkins.flo.pub/job/flowcommerce/job/lib-util/job/PR-188/6/scoverage-report/ |
Click here for code coverage report: https://jenkins.flo.pub/job/flowcommerce/job/lib-util/job/PR-188/7/scoverage-report/ |
Click here for code coverage report: https://jenkins.flo.pub/job/flowcommerce/job/lib-util/job/PR-188/8/scoverage-report/ |
Click here for code coverage report: https://jenkins.flo.pub/job/flowcommerce/job/lib-util/job/PR-188/9/scoverage-report/ |
Click here for code coverage report: https://jenkins.flo.pub/job/flowcommerce/job/lib-util/job/PR-188/10/scoverage-report/ |
Click here for code coverage report: https://jenkins.flo.pub/job/flowcommerce/job/lib-util/job/PR-188/11/scoverage-report/ |
No description provided.