Recursion help

I suck at recursion, and I need some help understanding where I’m
going wrong here.

I have a script that reads a file, then creates an excel form file.
Here’s the program.

puts “Enter file name”
filename = gets.chomp

puts “Enter carton size”
carton_size = gets.chomp

out_file = File.new(“C:/test/inner workings.xls”, “w+”)
out_file.write(“Carton number \t Carton quantity \t Card number \n”)

file_size = IO.readlines(filename).size
total_cartons = (file_size.to_f / carton_size.to_f).ceil
i = 0
sequence = 1

in_file = File.open(filename)

unless in_file.eof?

(i…(carton_size.to_i - 1)).each do |number|
record = in_file.readline
out_file.write("#{sequence}\t#{total_cartons}\t#{record[7, 19]}
\n")
end

sequence += 1
if sequence > total_cartons
break
end
end

The problem I am running into, is it works fine until the sequence +=
1 part.
the output looks like
1 \t 7 \t 1234 1234 1234
1 \t 7 \t 1234 1234 1235

then right before
2 \t 7 \t 1234 1234 1300
it says “Done!” and closes.

out_file.close
puts “Done!”

I hope this makes sense enough for me to get some help.

Thanks,
~Jeremy

Hi –

On Thu, 15 Nov 2007, [email protected] wrote:

carton_size = gets.chomp

break
then right before
2 \t 7 \t 1234 1234 1300
it says “Done!” and closes.

out_file.close
puts “Done!”

I hope this makes sense enough for me to get some help.

Do you want ‘until’ instead of ‘unless’?

David

David A. Black wrote:

Do you want ‘until’ instead of ‘unless’?

Oh man, whoops! haha. Yes, I do want that. I must be tired or
something to not catch something simple like that. Ok, well that
presents a new problem, because I know when it hits the end of the
file it will throw an error, but I will play with that and see what I
come up with. Thanks.

~Jeremy

Done! Works great, but I am always interested in seeing how it can be
better. Less code, cleaner, faster sort of thing. Here it is, if
anyone has any ideas on how I can make it better, please do tell.

puts “Enter file name”
filename = gets.chomp

puts “Enter carton size”
carton_size = gets.chomp

puts “Enter header description”
description = gets.chomp

out_file = File.new(“C:/test/#{description}.xls”, “w+”)
out_file.write("#{description}\n")
out_file.write(“Carton number \t Carton quantity \t Card number \n”)

file_size = IO.readlines(filename).size
total_cartons = (file_size.to_f / carton_size.to_f).ceil
i = 0
sequence = 1

in_file = File.open(filename)

until in_file.eof?

(i…(carton_size.to_i - 1)).each do |number|
if in_file.eof?
in_file.close
out_file.close
puts “Done!”
exit
end
record = in_file.readline
out_file.write("#{sequence}\t#{total_cartons}\t#{record[7, 19]}
\n")
end

sequence += 1
end

[email protected] wrote:

Done! Works great, but I am always interested in seeing how it can be
better. Less code, cleaner, faster sort of thing. Here it is, if
anyone has any ideas on how I can make it better, please do tell.

I’ll do my very best.

puts “Enter file name”
filename = gets.chomp

puts “Enter carton size”
carton_size = gets.chomp

carton_size = gets.to_i

puts “Enter header description”
description = gets.chomp

I generally prefer it if apps take such things as command line
parameters
instead of reading them from within. Makes scripting much less of a
hassle.
Also it just “flows” better when invoking the application. But I guess
that’s
a matter of preference.

out_file = File.new(“C:/test/#{description}.xls”, “w+”)
File.open(“C:/test/#{description}.xls”, “w”) do |out_file|
Btw: Why are you using “w+” instead of just “w”?

out_file.write(“#{description}\n”)
out_file.write(“Carton number \t Carton quantity \t Card number \n”)

Replace write with puts and then leave out the \n at the end.

file_size = IO.readlines(filename).size

in_file = File.readlines(filename)
file_size = in_file.size

total_cartons = (file_size.to_f / carton_size.to_f).ceil
i = 0

You never change i, so unless that’s a bug, you should leave out this
line.

sequence = 1

Leave that out too.

in_file = File.open(filename)

This too, because we already defined in_file above.

until in_file.eof?
in_file.each_with_index do |record, sequence|

(i…(carton_size.to_i - 1)).each do |number|

Change i to 0 if you leave out i above. Also you can leave out the to_i
because we already called it above.

if in_file.eof?
  in_file.close
  out_file.close
  puts "Done!"
  exit
end
record = in_file.readline

You can leave all that out because we’re iterating with each now and
out_file
is opened with open+block and as such doesn’t have to be closed (in_file
is
now an array in memory and not actually an opened file, so it needs no
closing either).

out_file.write("#{sequence}\t#{total_cartons}\t#{record[7, 19]}

\n")

out_file.puts “#{sequence+1}\t#{total_cartons}\t#{record[7, 19]}”

We need the +1 because each_with_index starts counting at 0 while your

original code counted from 1.

end

sequence += 1

You don’t need this line anymore.

end

plus one more end.
Here’s my version of the code all in one place:
http://pastie.caboo.se/118428
(untested)

HTH,
Sebastian

oh, I thought it was just where you have an iteration inside another
iteration or like nested loops or something. Hmm. Thanks for the help.

I tried your version of the program. It seems to go into an endless
loop, or just takes a REALLY long time. I stopped it after 30
seconds.

Honestly, I don’t really understand a whole lot of this. I just kind
of throw things together until they work, but I am getting a lot
better at understanding, which is why I like to see if people can make
my program better. I can then ask why they did the things they do.

So, I see why you would use “w” and not “w+”, but why do you use the
ARGV[0] ||?
I know that ARGV is an array, but I don’t know what it is used for, or
why you would use it instead of making your own array. Also, why would
you assign the filename the user input or ARGV[0]?

The thing with the each_with_index is that the second record will have
a sequence of 2, and it should have a sequence of 1. So sequence only
goes up to the total carton size. It is a ?? of ?? cartons i.e. 1 of
12 - 12 of 12.

This stuff gives me a good head start though, thanks for the
information.

~Jeremy

On Nov 15, 10:15 am, Sebastian H. [email protected]

[email protected] wrote:

I suck at recursion, and I need some help understanding where I’m
going wrong here.

You’re not using recursion at all in your code. Recursion is if a method
invokes itself.

[email protected] wrote:

oh, I thought it was just where you have an iteration inside another
iteration or like nested loops or something. Hmm. Thanks for the help.
“To understand recursion, one must first understand recursion.”

Or alternatively,

“To understand recursion, ask someone standing closer to Douglas
Hofstadter to explain it.”

:slight_smile:

[email protected] wrote:

I tried your version of the program. It seems to go into an endless
loop, or just takes a REALLY long time. I stopped it after 30
seconds.

As I said, I didn’t test the code, but I really can’t see where it would
go
into an endless loop. I only have each-loops in there.
If you give me a sample input file and a specification how the resulting
output file should look like I could go and debug.

So, I see why you would use “w” and not “w+”, but why do you use the
ARGV[0] ||?

As I said, I prefer command line arguments to reading from STDIN.
ARGV[0]||ask_for takes the first command line argument or, if there
was no first command line argument, asks the user for input.

I know that ARGV is an array, but I don’t know what it is used for

Command line arguments.

or
why you would use it instead of making your own array.

Your own array would only contain elements you put in it. ARGV contains
the
command line arguments.

Also, why would
you assign the filename the user input or ARGV[0]?

So the script doesn’t ask for user input if the filename was already
specified
as a command line argument.

The thing with the each_with_index is that the second record will have
a sequence of 2, and it should have a sequence of 1.

Your sequence variable started at 1, too, and was incremented by 1 at
every
iteration. The each_with_index with sequence+1 should behave exactly
like
that.

HTH,
Sebastian