Skip to content
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

Thusand separator in data sent/received #247

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

nmingotti
Copy link

. The proposed change adds a default thousand separator in the "SENT" and "RECEIVED" columns.
. Thousand separator improve quick data readability
. The thousand separator is character: "," , independently from user locale
. The change reduces the number of decimal digits to "1". This is to retain the author original decision to include them.
. When the number is too big to fit a column the "ERR:tooBig" is shown, inviting the user to change scale and see data in the proper format.

Copy link
Owner

@raboof raboof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't strongly care about thousands-separators, but I'm not opposed to it either.

I think we should keep the number of decimals to '3'.

I see some formatting problems. Rather than pointing them out individually, I'd suggest you 'make format'. I have done that for the main branch as well, as it seems we forgot this for some recent PR's. This might introduce some merge conflicts, sorry about that.

src/cui.cpp Outdated
Comment on lines 70 to 71
const int NUMBER_OF_DECIMALS_SENT = 1;
const int NUMBER_OF_DECIMALS_RECEIVED = 1;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you think showing one decimal is better than showing three? Three seems to make more sense to me.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, a few collected thoughts on decimals:
. I found nethogs a few days ago and really like it. But, the first thing that was not clear to me on the first run was "that dot is to separate thousands or decimals?". Maybe "B" stands for bit ! etc. etc. A 3 digits block makes (at least me) immediately think of thousand separator. The dot does not help, in central Europe this is a thousand separator.
. In general, it is of scarce interest to show even one decimal for numbers greater than, say 50. It is more noise than information since the error you introduce leaving out the decimal is very little. Still, it is noise that eats a lot of column space.
. Consider "top", there, where decimals are informative it is only 1 decimals, for magnitudes that are expected to be less than 100 most of the time.
. Column are very scarce resource, so if we keep long numbers (no automatic udm adjustment) the choice is between 2 decimals or 2 thousand separators. My preference is to read well the big numbers and forget about the tiny digits dance.

src/cui.cpp Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants