Skip to main content
added 2 characters in body
Source Link
Mast
  • 13.8k
  • 12
  • 57
  • 127

As mudasobwa's comment says, youryou're 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

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

As mudasobwa's comment says, you're 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
deleted 74 characters in body
Source Link
Flambino
  • 33.3k
  • 2
  • 46
  • 90

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

You're checking if input iscontains 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.

It also checks for the character(s) not being anywhere in the string. SoEven if I type exit or quit or whatever else to stop the task, it'll still happily delete stuff, since those strings do not contain an "n". 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?$^y$/i
      FileUtils.rm_r("tmp/letter_opener")
    else
      puts "Aborting"
    end
  end
end

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

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

It also checks for the character(s) not being anywhere in the string. So if I type exit or quit or whatever else, it'll still happily delete stuff, since those strings do not contain an "n". 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 gets.chomp =~ /^y?$/i
      FileUtils.rm_r("tmp/letter_opener")
    else
      puts "Aborting"
    end
  end
end

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
added 155 characters in body
Source Link
Flambino
  • 33.3k
  • 2
  • 46
  • 90

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

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

It also checks for the character(s) not being anywhere in the string. So if I type exit or quit or whatever else, it'll still happily delete stuff, since those strings do not contain an "n". 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 gets.chomp =~ /^y?$/i
      FileUtils.rm_r("tmp/letter_opener")
    else
      puts "Aborting"
    end
  end
end

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

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

It also checks for the character(s) not being anywhere in the string. So if I type exit or quit or whatever else, it'll still happily delete stuff, since those strings do not contain an "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 gets.chomp =~ /^y?$/i
      FileUtils.rm_r("tmp/letter_opener")
    else
      puts "Aborting"
    end
  end
end

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

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

It also checks for the character(s) not being anywhere in the string. So if I type exit or quit or whatever else, it'll still happily delete stuff, since those strings do not contain an "n". 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 gets.chomp =~ /^y?$/i
      FileUtils.rm_r("tmp/letter_opener")
    else
      puts "Aborting"
    end
  end
end
Source Link
Flambino
  • 33.3k
  • 2
  • 46
  • 90
Loading