From: Sebastian Hungerecker <sepp2k@...>
Date: 2007-11-16T03:14:10+09:00
Subject: Re: Recursion help.

JeremyWoertink@gmail.com 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
-- 
NP: Fjoergyn - Requiem
Jabber: sepp2k@jabber.org
ICQ: 205544826