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

Move and rename images into separate folders

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

Problem

Basically, we came up with this task at work. I was wondering if there's a shorter and cleaner way of doing the following in some other language (or even improving what I did with Ruby).

We have a folder. In this folder we have files (images only, /w .jpg extension). The names of the images are random strings such as:

  • 3e3eaa0b927d0158e1b2479ce31000b3.jpg



  • 3e3eaa0b927d0158e1b2479ce31000b3_1.jpg



  • 99a2b9c3442e716fc486e46abcfcfb05.jpg



  • 99a2b9c3442e716fc486e46abcfcfb05_1.jpg



  • 99a2b9c3442e716fc486e46abcfcfb05_2.jpg & etc.



To make it clearer, those images were related to certain objects. An object had multiple images (not a specific amount). We have to get those images, read their name, and get the "unique" part.

Example: For images:

  • 3e3eaa0b927d0158e1b2479ce31000b3.jpg



  • 3e3eaa0b927d0158e1b2479ce31000b3_1.jpg



the "unique" part is "3e3eaa0b927d0158e1b2479ce31000b3"

And we have to create a folder with this name (3e3eaa0b927d0158e1b2479ce31000b3) and put all the images that match (3e3eaa0b927d0158e1b2479ce31000b3_*). Also we have to rename those images, so in every folder there must be one image with thumb_ followed by a random string & the other images must be with img_ prefix and again followed by a random string.

This is my try:

require 'securerandom'
folder = "storage_test/"
Dir.glob("#{folder}*.jpg") do |img|
  fn = File.basename(img,".*").gsub(/(\_\d)+/, '') #folder name
  prefix = (!File.exists?( "#{folder}#{fn}") ? '/thumb_' : '/img_') #prefix
  new_name = "#{folder}#{fn}/#{prefix}" + SecureRandom.hex(32) + File.extname(img) #new_name (of the image)
  Dir.mkdir("#{folder}#{fn}")  unless File.exists?("#{folder}#{fn}") #create the folder
  File.rename(img, new_name) #move
end


(I work on Windows)

I'm sorry I've probably misused the term random. By random I mean that I need to create a name that's unique to the certain folder. Actually, there aren't going to be more than 40-50 files in a folder, so I presumed that `SecureRandom.h

Solution

As mentioned in the comments, your code is pretty dense. A little whitespace would help.

Your code comments also seem a little off. Many could be avoided by simply using more descriptive names for things. And a comment like prefix = ... #prefix doesn't really help. I also wonder why a comment like #new name (of the image) has a parenthetical. It's like the comment has a comment.

As for the functionality, your code seems ok, but there's one potentially fragile bit: Deciding the file prefix. You check for the existence of a directory when deciding the prefix. If the directory exists, you assume it also contains a thumb_ file. But that's not necessarily given. It'd be better to check if there's a thumb_ file there.

I'd also use File.join for more robust path contatenation.

In general, I'd break the logic into a few methods to make the steps more explicit. I'm making two assumptions here: That the images are always ".jpg", and that destination directories will be in the source directory.

require "securerandom"

# Gets the leading hexadecimal part of a filename
def unique_id(filename)
  $1 if File.basename(filename) =~ /\A(\h+)/
end

# Is there a file named "thumb_*.jpg" in the given directory?
def thumbnail_exists?(directory)
  pattern = File.join(directory, "thumb_*.jpg")
  Dir[pattern].any?
end

# Get the destination directory for a file
def directory_for_file(filename)
  base = File.dirname(filename)
  id = unique_id(filename)
  File.join(base, id)
end

# Returns the path for where to move an image file
def destination_for_file(filename)
  directory = directory_for_file(filename)
  prefix = thumbnail_exists?(directory) ? "img" : "thumb"
  new_name = "#{prefix}_#{SecureRandom.hex(32)}.jpg"
  File.join(directory, new_name)
end

# Move files
source_pattern = File.join("storage_test", "*.jpg")
Dir[source_pattern] do |file|
  directory = directory_for_file(file)
  Dir.mkdir(directory) unless File.exists?(directory)
  new_path = destination_for_file(file)
  File.rename(file, new_path)
end


It's kinda overwrought for what it has to do, but that's sort of on purpose to present a starker alternative.

Code Snippets

require "securerandom"

# Gets the leading hexadecimal part of a filename
def unique_id(filename)
  $1 if File.basename(filename) =~ /\A(\h+)/
end

# Is there a file named "thumb_*.jpg" in the given directory?
def thumbnail_exists?(directory)
  pattern = File.join(directory, "thumb_*.jpg")
  Dir[pattern].any?
end

# Get the destination directory for a file
def directory_for_file(filename)
  base = File.dirname(filename)
  id = unique_id(filename)
  File.join(base, id)
end

# Returns the path for where to move an image file
def destination_for_file(filename)
  directory = directory_for_file(filename)
  prefix = thumbnail_exists?(directory) ? "img" : "thumb"
  new_name = "#{prefix}_#{SecureRandom.hex(32)}.jpg"
  File.join(directory, new_name)
end

# Move files
source_pattern = File.join("storage_test", "*.jpg")
Dir[source_pattern] do |file|
  directory = directory_for_file(file)
  Dir.mkdir(directory) unless File.exists?(directory)
  new_path = destination_for_file(file)
  File.rename(file, new_path)
end

Context

StackExchange Code Review Q#85563, answer score: 2

Revisions (0)

No revisions yet.