Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I wrote a short ruby script to generate various screenshots, and event though I am not completely new to ruby I rarely use it.

Full script (41 LOC): https://gist.github.com/1229115

The string substitutions irritate me. They don't look quite right - much too ugly for a beautiful and concise language like ruby. Am I overdoing it? What alternatives are there?

Here is the lines I am concerned about (for context, please take a look at the gist):

# assemble file name
fname = "#{directory}/#{prefix}-#{timestamp}-#{commit}-#{size[:width]}x#{size[:height]}.png"

# create command line stuff
call = "#{macruby_exe} #{snapper} \
        --width #{size[:width]} --height #{size[:height]} #{url} #{fname}"

# now call it
system call

# just to show the progress (and the file sizes)
puts "#{fname} (#{File.size?(fname) / 1024}kB)"

Just to illustrate the script, the output usually looks like:

screenies/screenshot-1316535022-0b5c7967887481deb17bcd9039cd3fe0ac4540d4-640x960.png (95kB)
screenies/screenshot-1316535022-0b5c7967887481deb17bcd9039cd3fe0ac4540d4-800x480.png (62kB)
screenies/screenshot-1316535022-0b5c7967887481deb17bcd9039cd3fe0ac4540d4-800x600.png (78kB)
   ^          ^          ^                          ^                       ^
   |          |          |                          |               ________|______
   |          |          |                          |              |               |
directory   prefix     timestamp                  commit    size[:width] x size[:height]
share|improve this question
add comment

2 Answers

up vote 5 down vote accepted

There are other ways you could write this but I'm not sure if you'd find any of them cleaner. One option is plain old sprintf. For example:

# assemble file name
fname = sprintf("%s/%s-%i-%s-%ix%i.png", directory, prefix, timestamp,
          commit, size[:width], size[:height])

You could shorten it a little by giving size its own to_s method:

class << size; def to_s; "#{self[:width]}x#{self[:height]}" end end

fname = sprintf("%s/%s-%i-%s-%s.png", directory, prefix, timestamp,
          commit, size)

...and perhaps create other convenience methods along the same lines. But still, very small gains.

Another option would be to make your own super-small template engine, something like this:

def tiny_subst template, bndg
  param_exp   = /:(\w+)[^\w:]+/i
  tmpl_params = template.scan(param_exp).flatten

  proc {
    out_str = template.clone

    tmpl_params.reduce(out_str) do |str, p|
      str.gsub ":#{p}", eval(p, bndg).to_s unless p.empty?
    end
  }
end

tmpl   = ':directory/:prefix-:timestamp-:commit-:size.png'
subber = tiny_subst(tmpl, binding)

directory = 'screenies'
prefix    = 'screenshot'
timestamp = Time.now.to_i
commit    = '0b5c7967887481deb17bcd9039cd3fe0ac4540d4'
size      = '640x960'

subber.yield # => screenies/screenshot-1316535022-0b5c7967887481deb17bcd9039cd3fe0ac4540d4-640x960.png

commit  = 'foobarbazquux'
size    = '9000x9000'

subber.yield # => screenies/screenshot-1316535022-foobarbazquux-9000x9000.png

So, that's kinda nice, but is it really worth it? Not for me, but I guess if you really hate "#{this}" it might be.

share|improve this answer
 
Thank you - an inspiring answer. I don't really hate #{this} but the mini-template seems nicer, I'll try it. –  miku Sep 22 '11 at 10:22
 
Well it's not really intended for use but if you find it useful I actually put it up on GitHub last night: github.com/jrunning/StupidLittleTemplate There are probably security implications to eval-ing variables in the local scope, so please don't use it in a production app, or at the very least sanitize user data until it sparkles before passing it in. –  Jordan Sep 22 '11 at 15:15
 
Great, I'll take a look. –  miku Sep 23 '11 at 12:28
add comment

Per your request (and drawing from your gist), more concise and readable is:

screensizes = [
#     width height
    [   640, 960  ], # iPhone 4
    [   800, 480  ], # wvga
    [   800, 600  ], # old school
    [  1024, 600  ], # netbook, WSVGA
    [  1024, 786  ], # old school, iPad
    [  1200, 800  ], # macbook
    [  1366, 786  ], # netbook
    [  1440, 900  ], # macbook pro
    [  1600, 768  ], # strange
    [  1680, 1050 ], # hires macbok
]

screensizes.each do |w,h|
# Assemble file name:
  f = "#{directory}/#{prefix}-#{timestamp}-#{commit}-#{w}x#{h}.png"

# Create command line stuff:
  call = "#{macruby_exe} #{snapper} --width #{w} --height #{h} #{url} #{f}"

# Now call it:
  system call

# Show progress and file sizes:
  puts "#{f} (#{File.size?(f) / 1024}kB)"
end
share|improve this answer
add comment

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.