patternjavascriptMinor
Counting newlines in a file
Viewed 0 times
filecountingnewlines
Problem
I've started to go through Node tutorials on nodeschool.io, and the second assignment was to write a program that will count the number of newlines in a file and print it out. My solution looks like this
It's correct, and works. Theirs is
While both are correct, mine is longer (more lines used because I declared variables at every step).
I am wondering, am I wrong in declaring the variables for every step and then working on the code as I go? I mean, it does take more space, but I have the feeling that it's a bit clearer what I'm doing, unlike theirs. Or am I mistaken and haven't really gotten to that level where I can safely write everything in a one line like they did?
Also am I using memory space by declaring too many variables?
This task is rather trivial, and mostly the code I've written so far is for web elements (carousels and whatnot), so I'm used to writing mostly jQuery code instead of pure JavaScript (declaring variables used in events and such). Does this have any impact in real life apps?
var fs = require('fs');
var file = process.argv[2];
var buff = fs.readFileSync(file);
var str = buff.toString();
var str_arr = str.split('\n');
var newline_length = str_arr.length-1;
console.log(newline_length);It's correct, and works. Theirs is
var fs = require('fs')
var contents = fs.readFileSync(process.argv[2])
var lines = contents.toString().split('\n').length - 1
console.log(lines)
// note you can avoid the .toString() by passing 'utf8' as the
// second argument to readFileSync, then you'll get a String!
//
// fs.readFileSync(process.argv[2], 'utf8').split('\n').length - 1While both are correct, mine is longer (more lines used because I declared variables at every step).
I am wondering, am I wrong in declaring the variables for every step and then working on the code as I go? I mean, it does take more space, but I have the feeling that it's a bit clearer what I'm doing, unlike theirs. Or am I mistaken and haven't really gotten to that level where I can safely write everything in a one line like they did?
Also am I using memory space by declaring too many variables?
This task is rather trivial, and mostly the code I've written so far is for web elements (carousels and whatnot), so I'm used to writing mostly jQuery code instead of pure JavaScript (declaring variables used in events and such). Does this have any impact in real life apps?
Solution
The two versions execute exactly the same steps,
the only difference is the number of intermediary local variables.
Which version is more clear can be a matter of opinion.
Beginners may find your verbose version more clear,
more experienced programmers may find it tedious.
The good thing about the second version is that the local variables mark bigger logical steps that are quite intuitive. With one weakness, the meaning of the
But actually it's neither, it's the number of newline characters.
By contrast, your version marks the steps in too much detail,
for example it's not particularly interesting that
That step could be collapsed, as it is a too specific implementation detail,
not something "logical".
I think the best of both worlds would be more like this:
Each step marks a bigger logical step, with slightly more natural variable names.
the only difference is the number of intermediary local variables.
Which version is more clear can be a matter of opinion.
Beginners may find your verbose version more clear,
more experienced programmers may find it tedious.
The good thing about the second version is that the local variables mark bigger logical steps that are quite intuitive. With one weakness, the meaning of the
lines variable is not immediately clear. If you say "lines", it might mean a collection, or the count of lines.But actually it's neither, it's the number of newline characters.
By contrast, your version marks the steps in too much detail,
for example it's not particularly interesting that
readFileSync(file) doesn't return a String and it needs to be converted.That step could be collapsed, as it is a too specific implementation detail,
not something "logical".
I think the best of both worlds would be more like this:
var fs = require('fs');
var path = process.argv[2];
var text = fs.readFileSync(path).toString();
var lines = text.split('\n');
var newlines_count = lines.length - 1;
console.log(newlines_count);Each step marks a bigger logical step, with slightly more natural variable names.
Code Snippets
var fs = require('fs');
var path = process.argv[2];
var text = fs.readFileSync(path).toString();
var lines = text.split('\n');
var newlines_count = lines.length - 1;
console.log(newlines_count);Context
StackExchange Code Review Q#131143, answer score: 8
Revisions (0)
No revisions yet.