Skip to content

Fix 108 - #109

Open
rianwouters wants to merge 2 commits into
nodeftpd:masterfrom
rianwouters:fix-108
Open

Fix 108#109
rianwouters wants to merge 2 commits into
nodeftpd:masterfrom
rianwouters:fix-108

Conversation

@rianwouters

Copy link
Copy Markdown

Comment thread lib/glob.js
var stats = [];
var total = list.length;
for (var i = 0; i < CONC; ++i) {
for (var i = 0; i < list.length && i < CONC; ++i) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd consider a comparison with Math.min() more readable. Fans of premature optimization could save the min() result in a variable. ;-)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fine with me as long as its also done at the other places where this construction is used.

Anyhow it will change as the CONC construction is not working properly anyway for synchronous implementations . :-)

Verzonden van mijn HTC

----- Reply message -----
Van: "M.K." notifications@github.com
Aan: "sstur/nodeftpd" nodeftpd@noreply.github.com
CC: "rianwouters" rianwouters@hotmail.com, "Author" author@noreply.github.com
Onderwerp: [sstur/nodeftpd] Fix 108 (#109)
Datum: vr, jun. 17, 2016 13:08

@@ -16,7 +16,7 @@ function statList(fsm, list, callback) {

var stats = [];
var total = list.length;

  • for (var i = 0; i < CONC; ++i) {
  • for (var i = 0; i < list.length && i < CONC; ++i) {

I'd consider a comparison with Math.min() more readable. Fans of premature optimization could save the min() result in a variable. ;-)


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
https://github.com/sstur/nodeftpd/pull/109/files/f90a4004d3b42385edd37998b8a8f178389914d4#r67494604

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