------ art_21085_5371967.1196730081408 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Content-Disposition: inline On Dec 3, 2007 4:55 PM, Judson Lester <nyarly / gmail.com> wrote: > > > On Nov 30, 2007 7:16 PM, Khurrum Ma <khurrum1 / gmail.com> wrote: > > > I don't always know the best programmer practices and it would help me > > > > Sure. > > > > class DuplicateMP3 > > def initialize() > > unless ARGV.length 2 > > puts "usage: duplicatemp3.rb c:\source\dir\ c:\destination\dir\" > > exit > > end > > @dir_source RGV[0] > > @dir_destination RGV[1] > > > > showallduplicates() > > > Meh. I'd pass source and dest to initialize directly. I'd call > showallduplicates() on the DuplicateMP3 object directly. If you really > want, add a class method like > > def self.do_it(src, dest) > mover ew(src,dest) > mover.showallduplicates() > end > > Maybe. I'm shaky on this actually needing to be a class, but I guess it > works okay... > > end > > > > def showallduplicates() > > directory dir_source > > duplicates ash.new { |h,k| h[k] ] } > > Dir.chdir(@dir_source) > > > > #Check if the file is a file then > > #Make a unique ID with the mp3 title and mp3 song length combined > > #Enter unique into the hash as a key > > #If unique key is already in the hash then move the file to destination > > folder > > > > Here's the biggest thing: look into using Find (in the standard library.) > It's almost always superior to Dir, unless you really don't want to recurse > at all. > > > > > Dir["**/*.mp3"].each do |file| > > if File.file?(file) > > Mp3Info.open(file) do |mp3| > > unique p3.tag.title.to_s + mp3.length.to_s > > I'd use [mp3.tag.title.to_s, mp3.length] instead. It's certainly a corner > case, but it'd be a shame if you had "song" that was 210 long treated as a > dupe of "song2", 10 long. > > > > if (duplicates.has_key?(unique)) > > puts "duplicate: #{file}. Moving..." > > movedupes(file) > > else > > duplicates[unique].push(file) > > Meh again. Use a regular hash, and replace this line with > "duplicates[unique] rue" since you don't ever use the values of the > duplicates hash. > > end > > end > > end > > end > > end > > > > def movedupes(file) > > > And the solution to your input problem is: > "source ile::join(@dir_source, file.to_s)" > If you use Find, it'll be a little easier here, and tricker in the > destination, because Find will give you the full path anyway. > > > source dir_source + file.to_s > > > Instead of File.basename, you'll want to use something else. If you're > using Find, you'll get reasonably full paths, so you might be able to do > something like > file.to_s.sub(%r{^#{Regexp::escape(@dir_source)}}, "") > which will find the source path and replace it with an empty string. I > don't know a better way work with the paths, honestly. > And you'd use File::join again here, as well. > > > dest dir_destination + File.basename(file.to_s) > > File.move(source, dest) > > puts "file moved" > > end > > > > end > > > > find uplicateMP3.new() > > Ultimately, I think you want to have this look like > if($0 __FILE__) > #Put the ARGV.length check here > find uplicateMP3.new(ARGV[0], ARGV[1]) > find.showallduplicates() > end > > > > > -- > > Posted via http://www.ruby-forum.com/. > > > > > > > -- > Your subnet is currently 169.254.0.0/16. You are likely to be eaten by a > grue. -- Your subnet is currently 169.254.0.0/16. You are likely to be eaten by a grue. ------ art_21085_5371967.1196730081408--