How to improve iteration

[Chas Conquest]

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)
File.rename(letters, ordinals)
puts "switched to ordinal"
else
File.rename(ordinals, letters)
puts "switch to letter"
end
end
end
end

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

<---end---->

 

[Hakusa]

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

 

[Sebastian Hungerecker]

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

 

[Chas Conquest]

Fantastic guys! Thanks very much.

 

[Robert Klemme]

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

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



robert


PS: Sorry to all who expected an #inject solution. ;-)