JavaScript program to count lines of Java code

I've implemented a small program in JavaScript to count the number of lines of a string of java source code.

This was done for one of the Code Kata exercise: http://codekata.com/kata/kata13-counting-code-lines/

Overall, the goal of this exercise is summarised as follow:


The mixture of line-based things (single line comments, blank lines,
and so on) with the stream-based block comments can make solutions
slightly ugly. While coding your solution, consider the structure of
your code, and see how well it fits the structure of the problem. As
with most of these kata, consider coding multiple alternative
implementations. Does what you learned on the first tries affect your
approach to subsequent ones?

To be honest, I've submitted this for an interview and was told that this "lacked fundamentals by allowing errors in the code, there were issues in code construct and there were issues in choice of techniques".

Here's my simple app's source (also available in CodePen):



`/**
* Return the number of lines of code of a Java source code.
* {@link http://codekata.com/kata/kata13-counting-code-lines/}
* @param {string} Java source code
* @return {number}
*/
function getJavaLineCount(source) {
var count = 0;
var patterns = {
spaces: /^\s+/,
singleLineComment: /^\/\/.*/,
multiLineComment: /^\/\((?!\\/)[\s\S])\\//,
code: /^.\s/
};

var readToken = function () {
var m, type;
if (m = source.match(patterns.spaces)) {
type = 'spaces';
} else if (m = source.match(patterns.singleLineComment)) {
type = 'singleLineComment';
} else if (m = source.match(patterns.multiLineComment)) {
type = 'multiLineComment';
} else if (m = source.match(patterns.code)) {
type = 'code';
} else {
throw new Error("Unknown code pattern!");
}
var token = m[0];
source = source.substr(token.length);

Sorry, don't really have the time to do a comprehensive review (maybe later I'll edit!) but for now:

Your code actually fails on some cases.

The way that you're finding whether a line counts as a LOC or not is too greedy. (Alternatively, your comment regexes are too strict.) You're reading in basically the whole line as a line of code. This fails for relatively simple cases:

a/*
b*/

should be one line of code, but you treat it as two, since your code regex absorbs the /*. In fact, I'd argue that the way that you choose to count lines of code is odd; it took me a few reads to see what you were doing and find that error. The more natural way perhaps would be to strip spacing and comments, then count non-empty lines.

That also has the advantage of being way faster: you're calling String#substr() a lot, which is pretty expensive.

Aside from that, I only have a few comments:

• Keep spacing consistent. Choose either function() or function (); don't use both.

• Be consistent in your choice of indentation character. This is easier if you enable showing whitespace in your editor.

• Be consistent in string delimiters; don't switch between ' and " without a good reason.

• Since you're using a bare object, you can iterate over it with a for..in. If you're really concerned that somebody will put properties into the prototype of Object (which, by the way, should NEVER EVER happen), then add the hasOwnProperty check:

for (var tokenType in patterns) {
// if (patterns.hasOwnProperty(tokenType)) ...
if (m = source.match(patterns[tokenType])) {
var token = m[0];
source = source.substr(token.length);
//DEBUG
console.log('Found token (%s): %s', type, JSON.stringify(token));
return type;
}
}
throw new Error('Unknown code pattern!');


StackExchange Code Review Q#101978, answer score: 2