HiveBrain v1.2.0
Get Started
← Back to all entries
patternrubyMinor

Auto move files to a sub folder with Ruby

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
autowithsubmovefilesfolderruby

Problem

I started writing a script to handle my file downloads in osx because they quickly pile up and it gets time consuming to do this manually by drag and drop. I am looking forward to get some feedback.

require 'filewatcher'
require 'pathname'
require 'fileutils'

class FileAction

  def initialize(filename, event)
    @filename = filename
    @event = event
    @path = Pathname.new(@filename)
    @basename = @path.basename.to_s
    @base_path = "/Users/digerati"
    @dl_path = "#{@base_path}/Downloads"
    @dl_docs_path = "#{@base_path}/Downloads/Documents"
    create_folder_if_not_exist
    move_file
  end

  def create_folder_if_not_exist
    Dir.mkdir(@dl_docs_path) unless File.exists?(@dl_docs_path)
  end

  def move_file
    if(@event == :new)
      puts "Downloaded a document:" + @filename
      puts "Basename: " + @basename
      begin
        FileUtils.mv("#{@dl_path}/#{@basename}", "#{@dl_docs_path}/#{@basename}")
      rescue => e
        puts "+ Error: #{e.message}"
      end
    end
  end
end

@doc_files = ["pdf", "docx"].collect { |f| "~/Downloads/*.#{f}" }
FileWatcher.new(@doc_files).watch do |filename, event|
  FileAction.new(filename, event)
end

Solution

The constructor, initialize should only prepare the state of the object but not execute methods like move_file. You could call FileAction.new(filename, event).move_file instead.

You use a lot of instance variables that could replaced by local variables and method arguments. It follows the principle of minimum scope. That makes your code more functional. As result the methods become independ of the context of the class.

The class is not necessary since you have no real state to encapsulate.

FileWatcher.new(@doc_files).watch do |filename, event|
  move_file(filename) if event == :new
end


Hope it helps.

Code Snippets

FileWatcher.new(@doc_files).watch do |filename, event|
  move_file(filename) if event == :new
end

Context

StackExchange Code Review Q#120020, answer score: 2

Revisions (0)

No revisions yet.