How to improve iteration

Ruby
1

Hi,

Could you please point out how I can make this code more
compact/cleaner/smarter???
Gotta be a block…I just don’t see how to connect it.

Gratefully,

Chas

<------Switch File Names—>

ar = [“a.txt”, “b.txt”, “c.txt”, “d.txt”, “e.txt”]
br = [“first.txt”, “second.txt”, “third.txt”, “fourth.txt”, “fifth.txt”]

length = ar.length
class FileNameSwap
def switch( letters, ordinals, length)
for i in 0…length -1
if File.file?(letters[i])
File.rename(letters[i], ordinals[i])
puts “switched to ordinal”
else
File.rename(ordinals[i], letters[i])
puts “switch to letter”
end
end
end
end

switcher = FileNameSwap.new
switcher.switch(ar,br, length)

<—end---->

2

On 5/28/07, Chas C. [email protected] wrote:

  File.rename(letters[i], ordinals[i])

switcher.switch(ar,br, length)

<—end---->

Sure, assuming ar and br from above, and that they’re the same length,
you
can:

ar.zip(br).each do |letter, ordinal|
File.rename(letter, ordinal) rescue File.rename(ordinal, letter)
end

Or you can use the same construct (zip/each) with the if File.file?
conditional block, whichever you find more understandable.

-Scott

3

Chas C. wrote:

Could you please point out how I can make this code more
compact/cleaner/smarter???

I’ll try:

ar = [“a.txt”, “b.txt”, “c.txt”, “d.txt”, “e.txt”]
br = [“first.txt”, “second.txt”, “third.txt”, “fourth.txt”, “fifth.txt”]

class FileNameSwap

I don’t know why you chose to make a class for this, but since you did

I’ll

at least store the arrays as instance variables

def initialize(letters,ordinals)
@letters=letters
@ordinals=ordinals
end

def switch
@letters.zip(@ordinals).each do |letter, ordinal|
if File.file? letter
File.rename(letter, ordinal)
else
File.rename(ordinal, letter)
end
end
end
end

switcher = FileNameSwap.new(ar,br)
switcher.switch

HTH,
Sebastian H.

4

Fantastic guys! Thanks very much.

5

Instead of doing for…in and use i as an index, you could do
letters.each…do…|letter| and there’s no index. letter is the
extracted index.

The only problem would be the ordinals[i] which could stay as it is
and note your own iterations (not recommended) or do, instead of
letters.each, do letters.each_index do |i| and little would change.

Both suggestions would rid you of needing the length argument, but to
be honest that’s the only real disadvantage to your algorithm. It’s
pretty much fine how it is. You should only change it if it’s really
hard to find length in my opinion.

6

On 28.05.2007 19:47, Sebastian H. wrote:

I don’t know why you chose to make a class for this, but since you did I’ll

    File.rename(letter, ordinal)
  else
    File.rename(ordinal, letter)
  end
end

end
end

switcher = FileNameSwap.new(ar,br)
switcher.switch

And a strange variant of this:

def switch(letters, ordinals)
letters.zip(ordinals).each do |a|
File.rename((File.file? a[0] ? a : a.reverse))
end
end

:slight_smile:

robert

PS: Sorry to all who expected an #inject solution. :wink: