MY FIRST I/O! Instructions are confusing - program should NOT behave like `wc -l`

[Sequoia]

I keep getting a result that's expected+1. I'm stripping empty lines & removing \rs just to be safe:

var fs = require('fs');
var filename = process.argv[2];
var fileBuffer = fs.readFileSync(filename);

//get # of lines in `contents` Buffer
var lines = fileBuffer.toString().split("\n");
lines.forEach(function(line,index){//remove \r's
    lines[index] = line.replace(/\r/,'');
});
lines = lines.filter(function(elem){ //cast out empty lines
    return elem.length > 0;
})
//console.log(filename);
console.log(lines.length);

The issue is that because the test file is generated anew each time & has lots of long lines, it's not easy to tell what's wrong-- reading the file into an editor is not possible :question: and logging it produces too much output.

1. What am I doing wrong?
2. How should I have gone about debugging this?

[timoxley]

There's a hint in the problem description:

Using this method you'll end up with an array that has one more element than the number of newlines.

[Sequoia]

@timoxley ah you're right- I skimmed over this.

This is confusing because the top of the description gives the example cat file | wc -l. My code:

smcdowell@SMCDOWELL0L5 ~/learnyounode
$ cat babysteps.js | wc -l
      6

smcdowell@SMCDOWELL0L5 ~/learnyounode
$ node firstio.js babysteps.js
6

I see now that it says "print the number of newlines" but in my opinion the reference to wc -l should be removed: it's confusing as wc -l does not count the number of newline characters, but the number of lines.

I'd like to humbly propose either a. removing the reference to wc because it's slightly misleading or b. changing the test so it actually counts lines rather than newlines (counting newline characters rather than actual lines is a somewhat peculiar request in the first place IMO)

Thanks for the response! Let me know if you'd like me to attach a PR for one of my proposed changes or I guess go ahead and close the issue. :spaghetti:

[Sequoia]

OK I found the issue:

• Some text editors (I'm using vim in this case) append a \n character to each line. Example:
• Some text editors do not append a newline to the end of the final line (for example notepad++). Boganipsum, the package used to generate the dummy text for this package, follows this pattern (no newline char on final line).

This means that depending on which text editor you use to create the file, the number of new lines will be different. Using the code found here:

D:\Users\smcdowell\learnyounode> node .\firstioSOLUTION.js .\notepad.txt
2
D:\Users\smcdowell\learnyounode> node .\firstioSOLUTION.js .\vim.txt
3

This fact makes counting newlines a hard to test task (especially for beginners) without knowing a lot about one's editor & knowing the behavior of bogenipsum. With this in mind I'd like to recommend that this lesson be changed to count lines, not newline characters, as this is much more straightforward, less likely to cause confusion, and requires less knowledge of the underlying method used to generate the test data. I'll attach a pull request later tonight.

[rvagg]

@Sequoia I'm happy for you to open a pull request to change the wording and/or behaviour and we can discuss from there. Perhaps the way forward is to provide a code snippet that's portable, e.g.: contents.split(/[\r\n]{1,2}/).length

[timoxley]

or just split on \n take the length and call it a day.

[timoxley]

or, even better. just count words and split on \s.

[Sequoia]

@rvagg well the cr``lf vs. lf isn't actually the issue, it's "no newline at end of file," so to speak. Basically, in order to make the practice work on the file provided and on a file the user creates, a normalization step must be included which ensures there are either always or never new lines at the end of the input.

Because adding this step might complicate the program and doesn't pertain to the lesson itself, file i/o, I believe the simplest approach here is to just add a note that while some editors put a new line at EOF and some don't, "The file used to test your answer will not have a trailing newline character". That way the user knows what they need to know to complete the task.

[rvagg]

Thanks @Sequoia, as I said I'd welcome a pull request if you think you can improve on the wording, just go here and you can even edit in your browser.

The problem statement references the wc command that people not on Windows will likely be familiar with and it doesn't count the last newline, so you should get the same number using your solution as you do running wc.

Also, if you think you can come up with a better introductory I/O example that can be both sync and async then I'm all ears! I don't like that the semantics of this problem get in the way of the important part of the problem. We originally had it counting newline characters but then changed it to counting lines in the same way as wc but ...

[Sequoia]

Hopefully this addresses it sufficiently!!

well hot dog...

smcdowell@SMCDOWELL0L5 ~/learnyounode
$ cat vim.txt | wc -l && cat notepad.txt | wc -l
      3
      2

I guess the behavior of the solution file does match wc -l! I guess I need to fix up my PR in that case. Will do tomorrow.

[Sequoia]

alright PR is fixed up, removed the bit about it not behaving like wc.

[claudiopro]

+1 to @sequoia's comment. Description misleads savvy coders. It took me a while to realize that the exercise was expecting contents.split('\n').length - 1.

[timoxley]

@Sequoia merged https://github.com/rvagg/learnyounode/commit/7fc28fc0a1a5bdc382ebc6706b633151ab6c73cb