8
\$\begingroup\$

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

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 - 1

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?

\$\endgroup\$
5
  • 2
    \$\begingroup\$ Just to mention this line: " I'm used to writing mostly jQuery code instead of pure JavaScript". Please stop using jQuery and go learn JavaScript. You're only going to hurt yourself in the long run by relying on jQuery as a crutch; what you wrote there sounds like you don't actually know JavaScript, just jQuery. \$\endgroup\$
    – Dan
    Jun 5 '16 at 10:29
  • 1
    \$\begingroup\$ Well I know JavaScript, but when styling web pages it's a lot easier to use jQuery then pure JavaScript. I know that there are tons of JS stuff that I don't know, that's why I'm starting to learn Node and with it I'll expand my JS knowledge as well :) \$\endgroup\$
    – dingo_d
    Jun 5 '16 at 10:31
  • \$\begingroup\$ @DanPantry -100. You should totally drop that and use jQuery. \$\endgroup\$
    – gcampbell
    Jun 5 '16 at 11:37
  • 1
    \$\begingroup\$ One other note, in their example the variable lines is poorly named, because it suggests that it's an array of the lines themselves, rather than a count of the number of lines. numberOfLines or numLines or linesCount, etc would be more accurate. \$\endgroup\$
    – Jonah
    Jun 5 '16 at 16:27
  • \$\begingroup\$ Your version is better simply by virtue of your finding it easier to understand today. That increases the likelihood that you will understand it in six months. For what it's worth, I find it easier to understand too...but I'm not that bright. \$\endgroup\$ Jun 6 '16 at 3:59
8
\$\begingroup\$

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 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.

\$\endgroup\$
4
  • \$\begingroup\$ So basically it all comes down to experience - inexperienced user (me) needs more verbosity, while experienced ones don't. Is there any impact on having too many local variables declared, performance wise? \$\endgroup\$
    – dingo_d
    Jun 5 '16 at 11:25
  • 1
    \$\begingroup\$ @dingo_d Think in terms of big logical steps. Something that a less technical person like a business analyst would understand. If you do like that, then the verbosity can be helpful. The names you had like buff and str_arr were not particularly intuitive or natural. Naming is hard, and practice does help. In terms of performance, no difference whatsoever. \$\endgroup\$
    – janos
    Jun 5 '16 at 11:52
  • 1
    \$\begingroup\$ @dingo_d, everything janos said is right, but i would try to move away from the overly verbose style as quickly as possible. you will probably find that it's easier than you think, even as a beginner. \$\endgroup\$
    – Jonah
    Jun 5 '16 at 16:24
  • \$\begingroup\$ I'll have that in mind definitely. It's just that I've gotten used to have everything laid out clear when writing JS, that I do it automatically I guess. \$\endgroup\$
    – dingo_d
    Jun 5 '16 at 16:38

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.