6
\$\begingroup\$

What can I do with this program to improve its performance?

#!/usr/bin/env ruby
require 'open-uri'
print "URL: "
add = gets
puts "Info from #{add}"
begin
  open(add) do |f|
    puts "Fetching images..."
    puts "Fetching links..."
    puts "Fetching div tags..."
    puts "Fetching headers..."
    puts "Fetching forms..."
    puts "Processing..."
    img = f.read.scan(/<img/).length
    puts "\t#{img} images"
    f.close
  end
  open(add) do |f|
    links = f.read.scan(/<a/).length
    puts "\t#{links} links"
    f.close 
  end
  open(add) do |f|
    div = f.read.scan(/<div/).length
    puts "\t#{div} div tags"
    f.close
  end
  open(add) do |f|
    head = f.read.scan(/<h1/).length
    puts "\t#{head} 'h1 type' headers"
    f.close
  end
  open(add) do |f|
    form = f.read.scan(/<form/).length
    puts "\t#{form} forms"
    f.close
  end
rescue
  puts "An error occured, either you entered an invalid URL or your internet connection is messed up!"
end
\$\endgroup\$

1 Answer 1

5
\$\begingroup\$

Instead of opening, reading and closing the file all the time, you should read it once and then just use the string multiple times (this will also safe time and bandwidth).

img = f.read.scan(/<img/).length

This won't necessarily give you an accurate count of <img> tags. For example it will also count occurrences of <img that appear in comments or in scripts. If you want an accurate count, you should use an HTML parsing library. (Same applies to when you do the same thing to the other tags of course).

Also, as helpfully pointed out by Christopher Creutzig, in case of the a-tag your regex will also count <address tags or anything else that starts with a (and if there should ever be HTML tags whose name start with div or h1 etc., those will be miscounted as well). This can be fixed by adding a \b after the tag as Christopher suggested (so that the tag name can't be followed by another word-character). Using a proper HTML parsing library is still your best option though.

f.close

No need to close the stream when using open with a block. The stream will be closed automatically when the block ends.


I'd also recommend to put the tag-counting code into a method. This way you don't need to repeat the same code for each tag.

If you're okay with changing the output, so that it's uniform for all tags, you might even turn the whole thing into a loop, so there's no repetition at all. Something like this:

%w(img a div h1 form).each do |tag|
  count = contents.scan(/<#{tag}\b/).length
  puts "\t#{count} #{tag} tags"
end
\$\endgroup\$
1
  • 2
    \$\begingroup\$ And for good measure, make that /<#{tag}\b/ so <address> is not counted as an a tag. \$\endgroup\$ Commented Apr 10, 2012 at 17:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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