4
\$\begingroup\$

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.hex(32) will generate unique enough filename that will not cause overwriting in the same folder.

More detailed example:

Example folder: "storage_test"

And this is a part of its content:

example

I've grouped the files that should go in the same folder. I highlighted that part from the file's name that should be used as folders name.

So after executing the code "storage_test" will have:

  • 091afecdc49a8e8e581f11f2f1bfea8f_1/thumb_unique(to the folder)name.jpg
  • 091afecdc49a8e8e581f11f2f1bfea8f_1/img_unique(to the folder)name.jpg
  • 091afecdc49a8e8e581f11f2f1bfea8f_1/img_unique(to the folder)name.jpg
\$\endgroup\$
9
  • 2
    \$\begingroup\$ By "random", do you mean "cryptographically unguessable", or do you mean "arbitrary enough to guarantee distinctness"? \$\endgroup\$ Apr 1, 2015 at 8:15
  • \$\begingroup\$ More like "arbitrary enough to guarantee distinctness." \$\endgroup\$
    – FakeHeal
    Apr 1, 2015 at 9:15
  • \$\begingroup\$ I'm a little confused by the thumb vs img prefixing. It seems that the first file you find is simply named as the thumbnail, correct? So it's "random" what image file ends up as the thumbnail? \$\endgroup\$
    – Flambino
    Apr 1, 2015 at 9:19
  • \$\begingroup\$ "thumb_ followed by a random string & img_ followed by a random string", does that implicate that the random strings are different? Why not keep the mapping and just have one random string with 2 different prefixes. Or did I read your question wrong? \$\endgroup\$
    – DJanssens
    Apr 1, 2015 at 9:44
  • 1
    \$\begingroup\$ That's a pretty dense snippet of code you've got there. \$\endgroup\$
    – RubberDuck
    Apr 2, 2015 at 11:02

1 Answer 1

2
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.