1
\$\begingroup\$

I'm trying to remove the tmp/letter_opener folder. Everything works fine, just wondering if there's a better way to write it. My next step is to write some tests, to double check that I can only accept those inputs (currently failing)

#my_rake_task.rake

namespace :cleanup do

desc 'Deletes the emails inside tmp/letter_opener folder.'
task letter_opener_emails: :environment do

start_time = Time.current
Rails.logger.info "Task starting at #{start_time}."
puts "Task starting at #{start_time}."

print 'About to remove the tmp/letter_opener folder. Press [Nn] to abort. Press [Yy] to continue.'
option = STDIN.gets.strip

case option #[FIXME]: Not deleting the folders
  when /[^Nn]/ then FileUtils.rm_r(Dir.glob('tmp/letter_opener/*'))
    puts 'Directory contents removed'
  when /[^Yy]/ then 'Exiting the task now.' 
    # abort_message
end
end
\$\endgroup\$
  • 1
    \$\begingroup\$ Sidenote: option is handled in exactly opposite way. \$\endgroup\$ – Aleksei Matiushkin Mar 11 '16 at 13:52
2
\$\begingroup\$

As mudasobwa's comment says, your using the case statement backwards. And wrongly.

You're checking if input contains letters that are not n or N, and if that's true, you delete stuff. So I could type foobar, and it'd delete files. Bad.

Even if I type exit or quit or whatever else to stop the task, it'll happily delete stuff. Even typing cancel would be interpreted as "yes, please delete stuff", since - though it contains an "n" - it contains lots of letters that are not "n".

You also have a (dead) branch for not typing y or Y, though that makes no sense. And you don't have an else branch - not that you need one, but it'd make more sense.

You have a fixme you haven't fixed. Your indentation is missing or wrong. Your message ("About to remove the tmp/letter_opener folder.") doesn't match what the code does.

In all, it's not great.

All you need is this:

namespace :cleanup do
  desc 'Deletes the emails inside tmp/letter_opener folder.'
  task letter_opener_emails: :environment do 
    puts "Remove tmp/letter_opener? [yn]"
    if STDIN.gets.chomp =~ /^y$/i
      FileUtils.rm_r("tmp/letter_opener")
    else
      puts "Aborting"
    end
  end
end
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ thanks I'm still a newbie on this and I knew my regexp and the code logic was "passable", but definitely not great. Thanks once again. \$\endgroup\$ – intercoder Mar 13 '16 at 7:22

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.