How to improve iteration

ruby-talk
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---->

···

--
Posted via http://www.ruby-forum.com/.

2

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

···

On 5/28/07, Chas Conquest <chas@valley.net> wrote:

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---->

3

Chas Conquest 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 Hungerecker

···

--
NP: Sentenced - Blood & Tears
Ist so, weil ist so
Bleibt so, weil war so

4

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.

5

Fantastic guys! Thanks very much.

···

--
Posted via http://www.ruby-forum.com/.

6

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:

···

On 28.05.2007 19:47, Sebastian Hungerecker wrote:

Chas Conquest 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