Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Hi I wrote a small script as an answer for a stack overflow question, that counts lines of code and comments (in C and C++ style).

f = File.open("test.txt")

loc = 0
comments = 0
while line = f.gets
  if commented ||= line.match(/^\/\*/)
    commented = nil if line.match(/\*\/$/)
    comments += 1
  elsif line.match(/^\s*\/\//)
    comments += 1
  else
    loc +=1 unless line.match(/^\s*\n/)
  end
end

puts loc
puts comments

I'm just looking for some feedback/review.

share|improve this question

1 Answer 1

up vote 2 down vote accepted

Your code looks pretty good to me, only that it's very imperative. I know it's not for everybody's taste, but a functional approach on text processing is also feasible.

My take: you start by defining an object (which may be a hash) to store the state (needed for in intermediate iterations and the final result), and then fold (reduce/inject) over the input lines, returning the next state on each step.

There are some edge cases (lines with multiple or nested comment) which I won't consider, let's keep it simple. I'd write:

initial_state = {lines: 0, comments: 0, in_multiline_comment: false}
final_state = open("test.txt").lines.inject(initial_state) do |state, line|
  state_update = if state[:in_multiline_comment]
    {comments: state[:comments] + 1, in_multiline_comment: !line.match(/\*\//)}
  else
    case line 
    when /\/\*/
      {comments: state[:comments] + 1, in_multiline_comment: !line.match(/\*\//)}
    when /\/\//
      {comments: state[:comments] + 1}
    else
      {lines: state[:lines] + (line.strip.empty? ? 0 : 1)}
    end
  end
  state.merge(state_update)
end

Notice that you don't worry about stateful variables anymore, we just think "if I have this input and this current state, what output would I want?". Once you've covered all scenarios, the algorithm is done (hopefully leaving behind a declarative and maintainable code).

share|improve this answer
    
Thanks, definitively a whole different way of solving it. Plus a nice explanation of how it's done. –  Agush Dec 21 '12 at 23:17

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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