I've created a program for my job that unlocks spreadsheet files quickly and efficiently. What we use to have to do before this program is we'd have to go into the system, search for the file, kill the users process containing the file, unlock the file, then the user can finally get back into their spreadsheet. With my new program it first cd's into the directory containing the files, you enter the file number folio, it searches through a list of locked files using shell commands, then unlocks the specific one for the user.I'm looking for some critique on my work, I would really like to know what I can do better what I did wrong, etc..Source:#!/usr/local/bin/ruby  require 'fileutils'module Kernel    def ls_grep        `cd '/my/dir'`        `ls -la|grep -i .lock`        puts Enter Folio #        @input_folio = gets.chomp        lockfile = /my/dir.~lock.#{@input_folio}.ods#        if @input_folio =~ /^\d{7}/             if File.exist?( lockfile )                puts Unlock file?                input = gets.chomp.upcase                if input == 'Y'                     FileUtils.rm( lockfile )                     puts File unlocked.                else                    puts You went through that trouble for no reason...                end            else                puts <<-EDE.gsub(/^\s*>/, ' ')                 >                >Lockfile not found for Folio # #{@input_folio}                >                >If the file name doesn't match the folio number, get the file name                >and use that instead of the Foilio number...                >                EDE                ls_grep            end        else            puts <<-EDF.gsub(/^\s*>/, ' ')            >            >What part of 'Folio #' is hard to understand? 7 DIGITS            >            >If the file name doesn't match the folio number, get the file name            >and use that instead of the Folio number...            >            EDF            ls_grep        end    endendls_grepExample usage:Enter Folio #1234567 Lockfile not found for Folio # 1234567 If the file name doesn't match the folio number, get the file name and use that instead of the Foilio number...Enter Folio #erwe What part of 'Folio #' is hard to understand? 7 DIGITS If the file name doesn't match the folio number, get the file name and use that instead of the Folio number...Enter Folio #
Unlocking spreadsheet files quickly
ruby;file;bash;locking
Don't put your method inside Kernel.Rename ls_grep to something more descriptive, e.g. unlock_spreadsheet.The two shell commands (cd and ls|grep) aren't needed. The code doesn't depend on the current directory, and you're not using the commands' output.Don't use an instance variable (@input_folio) as a temporary variable inside a method. Make it a local variable.The regex matches anything that begins with sevent digits. Use /\A\d{7}\z/ instead in order to match against the whole string.Don't check if the file exists before deleting it, someone might delete it after you check and before you try to delete it. Instead, try to delete it and catch the possible exception.Don't use recursion to repeat the method. Use a loop.                puts <<-EDE.gsub(/^\s*>/, ' ')                 >                >Lockfile not found for Folio # #{@input_folio}                >                >If the file name doesn't match the folio number, get the file name                >and use that instead of the Foilio number...                >                EDERemove the call to gsub:                puts <<-EDE Lockfile not found for Folio # #{@input_folio} If the file name doesn't match the folio number, get the file name and use that instead of the Foilio number...                EDEYou may want to put those strings in global constants to avoid this decrease in indentation in the code.