Problem
This is a simple and short “Pretty Bytes” javascript function using the SI Decimal System for quantifying bytes.
The code does not use complex maths such as Math.pow()
or Math.log()
and mainly uses strings with an array.
The function uses the SI system of decimals as shown below (1 kilo = 1,000, 1 M = 1,000,000, etc.).
The number of decimal places is defaulted to 2 (no rounding is necessary) but can be modified on calling the function to other values. The common most used display is the default 2 decimal place.
The code is short and uses the method of Number String Triplets to first convert the input number (the integer) into a Number String Triplet, then into an array of triplets.
The following one-line code is used for converting an integer into a Number String Triplet so that the resulting string will always be made of multiple of 3’s by padding zero(s) to the left. It is the same concept used under this code review article: Simple Number to Words using a Single Loop String Triplets
Num = "0".repeat((Num += "").length * 2 % 3) + Num; // Make a Number String Triplet
The output is then made up of concatenating the first element of the array being the whole number and the 2nd element being the fraction (after chopping off for decimals) and adding the suffix letter.
Numbers below 1000 do not require the functional code and are cleared out first.
If there is a possibility that the input number may be a float, it should be converted to an integer first Num = Math.floor(Num);
before passing it to the function.
As the code is mainly for displaying size, no code is added for handling negative numbers or floats (not sure if one would give a size as ‘3 and a half byte’).
I welcome your code review.
Thanks
Mohsen Alyafei
/*********************************************************************
* @function : numberPrettyBytesSI()
* @purpose : Convert number bytes size to human readable format
* using the SI Decimal System.
* @version : 1.00
* @author : Mohsen Alyafei
* @date : 01 July 2020
* @param : {num} [integer] Number to be converted
* @param : {decimals} [integer] Deciaml places (defaul 2)
* @returns : {String} Pretty Number
**********************************************************************/
function numberPrettyBytesSI(Num=0, decimals=2){
if (Num < 1000) return Num + " Bytes";
Num = "0".repeat((Num += "").length * 2 % 3) + Num; // Make a Number String Triplet
Num = Num.match(/.{3}/g); // Make an Array of Triplets
return Number(Num[0]) + // Whole Number withou leading zeros
"." + // dot
Num[1].substring(0, decimals) + " " + // Fractional part
" kMGTPEZY"[Num.length] + "B"; // The SI suffix
}
//*********** tests ***********************
console.log(numberPrettyBytesSI(0)); // 0 Bytes
console.log(numberPrettyBytesSI(500)); // 500 Bytes
console.log(numberPrettyBytesSI(1000)); // 1.00 kB
console.log(numberPrettyBytesSI(15000)); // 15.00 kB
console.log(numberPrettyBytesSI(12345)); // 12.34 Kb
console.log(numberPrettyBytesSI(123456)); // 123.45 kb
console.log(numberPrettyBytesSI(1234567)); // 1.23 MB
console.log(numberPrettyBytesSI(12345678)); // 12.34 MB
console.log(numberPrettyBytesSI(123456789)); // 123.45 MB
console.log(numberPrettyBytesSI(1234567890)); // 1.23 GB
console.log(numberPrettyBytesSI(1234567890,1)); // 1.2 GB
console.log(numberPrettyBytesSI(1234567890,3)); // 1.234 GB
Solution
Here are the issues that I have identified. Overall this is a nice approach and I am considering using a string based approach like this to compute size strings in my software.
- Overwrought comment. I would prefer only to see the “purpose” sentence here as a plain comment.
- Function body missing indentation.
- Indentation that is found in multiline return expression consists of 7 spaces. This really should be either 2 spaces, 4 spaces, or tabs.
- First argument
Num
is capitalized. This is not a good code style because variables should never be capitalized. Usually only classes and such things are written with this kind of case. - That variable
Num
is an argument to the function and should not get shadowed. Meaning it’s nicer if you treat it like a const var and make a new variable if you need to store something else. It is shadowed twice, once to be a string, and again to be an array (regex match result). - The first arg
Num
should probably not have a default value. What meaningful value does it add for this function to be callable without args to produce0 Bytes
? - Several typos (
Deciaml
,defaul
,withou
). - A comment
// dot
for the code"." +
, which is egregious (do not insert a comment for code that is self-explanatory). - Suggest the return expression to be an interpolated JS template string
- Questionable use of
+= ""
to coerceNum
(a number input) into a string. Prefer to useString()
here. - Use of regex match would not be my first choice. It does work and is pretty concise, however we only ever use the length (number of groups of 3 digits) and the first group of 3 digits. So we really only need to grab the first 3 digits from
Num
and grab its length divided by 3, and do not need to run a whole regex match. - Your comments for the results from your tests are not consistent in case, e.g. neither
kb
norKb
would be possible for it to output. - The code does not (but could be easily adjusted to) work properly for
decimals
greater than3
. - The reasoning of
length * 2 mod 3
to produce the number of digits that we add as leading zeros to yield a string whose length is a multiple of 3 is not clear. As such, this code has maintainability issues. The reasoning behind this math should be a comment here.
I never would have imagined that there would be this many things to nitpick in this code but such are the high standards we try to hold ourselves to to attain “clean code”. The list does not include all of the good things that you’re doing in here. Especially the very simple approach to testing the code, which I’m a big fan of.
My final code:
// Convert a size in bytes to a human readable SI format
function bytesSI(v, decimals = 2) {
if (v < 1000) return v + " bytes";
const vs = String(v),
digits = vs.length,
prefix_group = Math.floor((digits - 1) / 3),
dot = ((digits - 1) % 3) + 1;
return `${vs.slice(0, dot)}.${vs.slice(dot, dot + decimals)} ${
" kMGTPEZY"[prefix_group]
}B`;
}
// tests
console.log(bytesSI(0)); // 0 Bytes
console.log(bytesSI(500)); // 500 Bytes
console.log(bytesSI(1000)); // 1.00 kB
console.log(bytesSI(15000)); // 15.00 kB
console.log(bytesSI(12345)); // 12.34 kB
console.log(bytesSI(123456)); // 123.45 kB
console.log(bytesSI(1234567)); // 1.23 MB
console.log(bytesSI(12345678)); // 12.34 MB
console.log(bytesSI(123456789)); // 123.45 MB
console.log(bytesSI(1234567890)); // 1.23 GB
console.log(bytesSI(1234567890, 1)); // 1.2 GB
console.log(bytesSI(1234567890, 3)); // 1.234 GB
console.log(bytesSI(1234567890, 4)); // 1.2345 GB
console.log(bytesSI(1234567890, 5)); // 1.23456 GB
console.log(bytesSI(1234567890, 6)); // 1.234567 GB
console.log(bytesSI(123456789012345678901, 5)); // 123.45678 EB
I would like to emphasize that I still don’t consider this done. I think that this line of code
dot = ((digits - 1) % 3) + 1;
and probably others do require some well-considered comments to make the code much more maintainable than it is in this state.