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

The words count and min to read seems wrong [fix added] #5

Closed
tankeryang opened this issue Jan 21, 2018 · 7 comments
Closed

The words count and min to read seems wrong [fix added] #5

tankeryang opened this issue Jan 21, 2018 · 7 comments
Labels

Comments

@tankeryang
Copy link

Environment:

  • node: 8.9.4
  • hexo: 3.4.4
  • hexo-cli: 1.0.4
  • NexT: 6.0.x

Problem description:
I‘ve install hexo-symbols-count-time and set it into NexT. It already worked. But the results seems to be wrong.
I'm chinese so I set AWL = 25 and WPM = 275. The result just like the following picture
opera _2018-01-21_224116_tankeryang github io

I'm sure that I didn't wrote so many words, and the time format makes me confused. Is that means 2 sec?

@ivan-nginx
Copy link
Member

ivan-nginx commented Jan 21, 2018

It's 0:02 — 0 hours, 02 minutes. Format is H:MM. See issue #4 for details.

@tankeryang
Copy link
Author

Thanks. I got it. But I still think the calculation was wrong >_<. I checked the ./next/layout/_macro/post.swig, and I found these code

_20180122134612

Then I found ./node_modules/[email protected]@hexo-symbols-count-time/index.js and tracked to ./node_modules/[email protected]@hexo-symbols-count-time/lib/helper.js. I saw these code

_20180122140715

Unfortunately, I’ve never wrote js code, I write python and C/C++. But I guess I can possibly understand what the code will do. But the code with the red line in the picture above I can't understand it. I don't know how hexo-util.stripHTML works, But why the symbolsCount calculates like that? And * 10 / 10 doesn't make sense.

@ivan-nginx
Copy link
Member

ivan-nginx commented Jan 22, 2018

This is a little bit difficult, yes. But when i write this code (in past year ~in Sep.) i'm trying to isolate and separate all possible functions and exports with minimum comments. And when u see small comments at the right u can guess what it mean if u take a look at the begining of this function. But let's go in order:

var getFormatTime = function(minutes) {
  var fHours = Math.floor(minutes / 60);
  var fMinutes = Math.floor(minutes - (fHours * 60));
  if (fMinutes < 1) {
    fMinutes = '01'; // 0:0 => 0:01
  } else if (fMinutes < 10) {
    fMinutes = '0' + fMinutes; // 0:9 => 0:09
  }
  return fHours + ':' + fMinutes;
};

This function give as result time in format as i say before: H:MM. Just from string in minutes. For example, if we get 127 minutes before this function, after the result we get 2:07.
1 comment mean minimal time cannot be 0:0, only 0:01 (minutes).
2 comment mean need to +0 to minutes if minutes <10 (1-9).

module.exports.symbolsCount = function(content) {
  var symbolsResult = getSymbols(content);
  return symbolsResult < 1024
    ? symbolsResult // < 999 => 100
    : Math.round(((symbolsResult / 1000) * 10) / 10) + 'k'; // > 999 => 1.1k
};

This function count symbols of current post from it's body. And your question is about « / 1000) * 10) / 10»:
If post body have < then 1024 symbols, then we just give this result as is (561 symbols, for example);
But if post have more then 1024 (5687 symbols for, example) symbols then we just round it to 5 and do plus 'k' string.
And as u can see only with Math.round procedure we / 10 and before this, we * 10.
Take a look here: Math.round(((symbolsResult / 1000) * 10) / 10):

  1. We do (symbolsResult / 1000) operation
  2. Result of this op we * 10
  3. With the result of previous op we do Math.round($previous_op / 10)

This mean we never get something like 5.68k as a result and always get only 5.7 to round with bigger digit (because 5687 is closer to 5.7 not to 5.6). If it will be 5617 — when will round to 5.6k. Try to remove * 10) / 10 and see what u get with this digits.

module.exports.symbolsCountTotal = function(site) {
  var symbolsResultTotal = getSymbolsTotal(site);
  return symbolsResultTotal < 1024024
    ? Math.round(symbolsResultTotal / 1000) + 'k' // < 999k => 100k
    : Math.round(((symbolsResultTotal / 1000000 * 10)) / 10) + 'm'; // > 999k => 1.1m
};

And this function, as u can see, it's like previous, but there is can be on whole site more then 999k symbols (in one post it can't be). So, this is double filter and u have same description as above, but only for count with millions symbols.


Hope i give answer on your question and always remember: in javascript all make sense. 😃

@tankeryang
Copy link
Author

tankeryang commented Jan 22, 2018

Wow! Thanks! Thanks for your patience! It's my first time to discuss with someone on github. So funny :)
I read your explanation, and read the code again. And I try to test some js code on W3School online. The result like this (left is code, right is running result):

opera _2018-01-23_023651_www w3school com cn

so

Math.round(((5687 / 1000) * 10) / 10)

and

Math.round(5687 / 1000)

is the same one, because Math.round()will return an int. And what you want to do for example, is to set 5687 turn to be 5.7k . You want to use 57 / 10 to get the result. So I think may be this is the correct code:

Math.round((5687 / 1000) * 10) / 10

you just type redundant parenthesis around ((5687 / 1000) * 10) / 10
Am I right?

And the code can be simplified like this:

Math.round(5687 / 100) / 10

In fact, I think the original code is okay too. And I guess may be in this function:

var getSymbols = function(content) {
  var util = require('hexo-util');
  var stripHTML = util.stripHTML;
  return stripHTML(content).length;
};

we have to do some calculation with stripHTML(content).length but not just return it, because I really didn't write so many words (>_<)

Thank you again! And I think, Python is the best :) (just a joke😃)

@ivan-nginx
Copy link
Member

ivan-nginx commented Jan 22, 2018

Yeah, u are right, something wrong here. I just run test and see what there is no 1.3k, just 1k:

  describe('Test symbolsCount > 1024', function() {
    var symbols = helper.symbolsCount('Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum. Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum. Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.');

    it('assert - 1k', function() {
      assert.equal(symbols, '1k');
    });

  });

I say what this code was written a long time ago and i don't remebmer why i made it exactly like that.

All of i remember idea was in:

If we get 567 symbols, then we return it as it — 567
If we get 5687 symbols, then we return rounded — 5.7k
If we get 56870 symbols, then we return without dot — 57k
If we get 568700 symbols, then we return without dot — 569k

And test not totally finished, so, need to finish this work.


we have to do some calculation with stripHTML(content).length but not just return it, because I really didn't write so many words (>_<)

stripHTML(content).length return count of symbols (not words) in post body.
It's just need to rename translates in NexT from Words to Symbols, bug added.


parseInt(568700 / 1000) + (Math.round((5687 % 1000) / 100) / 10) + 'k'

This code is a little bit slower i think, need to do with math simple as possible.


you just type redundant parenthesis around ((5687 / 1000) * 10) / 10
Am I right?

Yes! Here is bug. Thank u! 👍

-    : Math.round(((symbolsResult / 1000) * 10) / 10) + 'k'; // > 999 => 1.1k
+    : Math.round((symbolsResult / 1000) * 10) / 10 + 'k'; // > 999 => 1.1k
  7 passing (13ms)
  1 failing

  1) Hexo Symbols Count Time Test symbolsCount > 1024 assert - 1k:
     AssertionError: expected '1.3k' to equal '1k'
      at Context.<anonymous> (test/index.js:34:14)

And the code can be simplified like this:

Math.round(5687 / 100) / 10

And this is good, yeah! Minus 1 interation. I just cycled on >1024, so, i divide it on 1000 not on 100.


Pull requests are welcome!

ivan-nginx added a commit that referenced this issue Jan 22, 2018
@ivan-nginx
Copy link
Member

Ok, rounding fixed in eca327d

> mocha test --reporter spec


  Hexo Symbols Count Time
    Test symbolsCount function & check should / expect / assert
       should - 22
       expect - 22
       assert - 22
    Test symbolsCount < 1024
       Symbols: 569 => 569
    Test symbolsCount > 1024
       Symbols: 1337 => 1.3k
    Test symbolsCount > 10k
       Symbols: 10703 => 11k
    Test symbolsCount & symbolsTime
       assert (symbolsCount - 90)
       assert (symbolsTime without parameters (awl = 5, wpm = 200) - 0:01)
       assert (symbolsTime [awl = 5, wpm = 5] - 0:04)
       assert (symbolsTime [awl = 1, wpm = 50] - 0:02)


  10 passing (9ms)

@tankeryang
Copy link
Author

haha! seems I did a good job. May be I have to learn some js and font-end, so that I can make some pull request.
Thanks! you can close the issue now. 😃

@ivan-nginx ivan-nginx changed the title The words count and min to read seems wrong The words count and min to read seems wrong [fix added] Jan 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants