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

Collapsing Folder Trees with Ruby

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

Problem

This is a quick script I whipped up to collapse some folder trees down to individual folders. The specific situation that arose was my formerly-itunes collection (it's currently organized Music\artist\album\songs.(mp3|m4a) and I'd really prefer it to be Music\artist\song.(mp3|m4a)).

#!/usr/bin/ruby

require 'optparse'
require 'pp'
require 'fileutils'

### parsing command line options via optparse, nothing to see here
options={}
optparse = OptionParser.new do|opts|
  opts.on('-h', '--help', 
          'Display this screen') do
    puts opts
    exit
  end
  opts.on('-t', '--type a,b,c', 
          Array, 'Specify filetypes to flatten out (defaults to mp3,m4a)') do|t|
    options[:types] = t
  end
end
optparse.parse!

### the setup (ideally, this would be in a `let` statement)
origin = Dir.pwd
if not options[:types]
  options[:types] = ["mp3", "m4a"]
end

### the actual work
ARGV.each do |target|
  FileUtils.cd("#{origin}/#{target}")
  options[:types].each{|ext| `find -iname '*.#{ext}' -exec mv {} ./ \\\;`}
  `find -depth -type d -empty -exec rmdir {} \\\;`
end
FileUtils.cd(origin)


Am I doing anything eye-stabbingly wrong? Is there a better way of handling any of the above?

Specifically, I suspect that I could replace the find calls with native Ruby methods, but I can't see how without complicating it significantly. I also suspect that there's a better way of writing FileUtils.cd("#{origin}/#{target}"), but a cursory reading of Dir doesn't reveal anything. Any tips?

```
#!/usr/bin/ruby

require 'optparse'
require 'pp'
require 'fileutils'

### parsing command line options via optparse
options={:types => ["mp3", "m4a"]}
optparse = OptionParser.new do|opts|
opts.on('-h', '--help', 'Display this screen') do
puts opts
exit
end
opts.on('-t', '--type a,b,c', Array, 'Specify filetypes to flatten out (comma separated, defaults to mp3,m4a)') do|t|
options[:types] = t
end
end
optparse.parse!

### the actual work
ARGV.each do |target|
File

Solution

options={}
# ...
if not options[:types]
  options[:types] = ["mp3", "m4a"]
end


I would initialize options with the default values first, instead of checking whether the option is set afterwards. I.e.

options = {:types => ["mp3", "m4a"]}


options[:types].each{|ext| `find -iname '*.#{ext}' -exec mv {} ./ \\\;`}


This can be done with a glob. For this I'd first change options[:types] to be string (i.e. replace Array with String in the optparse code and use "mp3,m4a" instead of ["mp3", "m4a"] as the default. Then you can use Dir.glob with FileUtils.mv like this:

Dir.glob("**/*.{#{ ext }}") {|f| FileUtils.mv(f, ".")}



I suspect that I could replace the find calls with native Ruby methods, but I can't see how without complicating it significantly.

For the first one see above. However the second one would indeed be more complicated to replace, so I'd just stick with the find call.


I also suspect that there's a better way of writing FileUtils.cd("#{origin}/#{target}")

Yes, there is: cd optionally takes a block. When given a block, cd will cd into the given directory, execute the block, and then cd back. So you can get rid of origin and just do

FileUtils.cd(target) do
  ...
end


FileUtils.cd(origin)


Even without using the block version of cd the above line would be completely unnecessary. Changing the directory at the end of a script has no effect. Any cd performed in a process will never affect the parent process.

Code Snippets

options={}
# ...
if not options[:types]
  options[:types] = ["mp3", "m4a"]
end
options = {:types => ["mp3", "m4a"]}
options[:types].each{|ext| `find -iname '*.#{ext}' -exec mv {} ./ \\\;`}
Dir.glob("**/*.{#{ ext }}") {|f| FileUtils.mv(f, ".")}
FileUtils.cd(target) do
  ...
end

Context

StackExchange Code Review Q#2419, answer score: 5

Revisions (0)

No revisions yet.