Help me condence my code?

simonsaid · September 7, 2007, 2:31am

I know this could be more idiomatic to ruby.

it’s basically to turn Fifteen Thousand Useful Phrases by Grenville Kleiser - Free Ebook into an
array. the “_no_extras” refers to me having snipped the intro and
outro of the text outside of ruby. I still have to do something with
the “SECTION” fields and the “A”, “B”, etc. fields. (not to mention
some kind of linguistic parsing which would make f[rand(f.size)] + " "
f[rand(f.size)] + " " … link together in a coherent matter, but
that’s a little beyond me. any direction in this area would be kindly
appreciated though! I’m thinking of separating it into different text
files maybe. certain sections are almost whole sentences, they’re
grouped in all kinds of ways that will maybe help with this. no
long-term goal, really, just learning ruby and having fun.

anyhow, the sloppy newbie code is as follows:

f = File.read(“phrases_no_extra.txt”)
f = f.to_a
f = f.each { |x| x.chop! }
f.each_with_index { |x,y| # deletes the empty array items
if x.size == 0
f.delete_at(y)
end
}
f.each_with_index { |x,y| # deleting all but the last (which is
spread of two lines)
if x.include? “]” # of his comments
f.delete_at(y)
end
}
f.each_with_index { |x,y| # yes, this is me unable to recall
how to do “or” hahaha.
if x.include? “[”
f.delete_at(y)
end
}
f.delete_at(-1) # random whitespace item at the end from the
last quote

puts f[rand(f.size)]

simonsaid · September 7, 2007, 2:48am

Simon S. wrote:

I know this could be more idiomatic to ruby.

it’s basically to turn Fifteen Thousand Useful Phrases by Grenville Kleiser - Free Ebook into an
array. the “_no_extras” refers to me having snipped the intro and
outro of the text outside of ruby. I still have to do something with
the “SECTION” fields and the “A”, “B”, etc. fields. (not to mention
some kind of linguistic parsing which would make f[rand(f.size)] + " "
f[rand(f.size)] + " " … link together in a coherent matter, but
that’s a little beyond me. any direction in this area would be kindly
appreciated though! I’m thinking of separating it into different text
files maybe. certain sections are almost whole sentences, they’re
grouped in all kinds of ways that will maybe help with this. no
long-term goal, really, just learning ruby and having fun.

anyhow, the sloppy newbie code is as follows:

f = File.read(“phrases_no_extra.txt”)
f = f.to_a
f = f.each { |x| x.chop! }
f.each_with_index { |x,y| # deletes the empty array items
if x.size == 0
f.delete_at(y)
end
}
f.each_with_index { |x,y| # deleting all but the last (which is
spread of two lines)
if x.include? “]” # of his comments
f.delete_at(y)
end
}
f.each_with_index { |x,y| # yes, this is me unable to recall
how to do “or” hahaha.
if x.include? “[”
f.delete_at(y)
end
}
f.delete_at(-1) # random whitespace item at the end from the
last quote

puts f[rand(f.size)]

ok, here’s my first shot at a newbie to programming trying to solve
something like this. I hope this works for what you are trying to do. I
wish I would have had an example to play with before posting, but oh
well. I can only learn from this as well right? well here goes…

f = IO.readlines(“phrases_no_extra.txt”).each_with_index do |x, y|
if x.chop!.size.eql?(0)
f.delete_at(y)
end
if x.include?(“]”) or x.include?(“[”)
f.delete_at(y)
end
end.delete_at(-1)
puts f[rand(f.size)]

let me know how it goes!

~Jeremy

simonsaid · September 7, 2007, 3:04am

Simon S. wrote:

anyhow, the sloppy newbie code is as follows:

Write unit tests for it, then refactor it, one tiny change at a time,
passing all the tests after each change. If the tests fail, undo the
change.

simonsaid · September 7, 2007, 3:55am

Jeremy W. wrote:

end.delete_at(-1)

OMG! You can call methods in an end??? I have lived far to long in the
scalar world! When would you do such a thing?

simonsaid · September 7, 2007, 4:10am

On 9/7/07, Lloyd L. [email protected] wrote:

Jeremy W. wrote:

end.delete_at(-1)

OMG! You can call methods in an end??? I have lived far to long in the
scalar world! When would you do such a thing?

irb(main):004:0> [1, 2].collect do |i| i+3; end.join " : "
=> “4 : 5”

The method call is not on the “end” but on the result of the method
using the block that the “end”, well, ‘closes’. Very functional.

Best regards,

simonsaid · September 7, 2007, 4:25am

On 7 Sep 2007, at 10:55, Lloyd L. wrote:

Jeremy W. wrote:

end.delete_at(-1)

OMG! You can call methods in an end??? I have lived far to long
in the
scalar world! When would you do such a thing?

Remember do…end is just another way of saying {…}

If I need to filter things out of an array of data I often do things
like:

data = [1,2,3,4,5,6,6,7,7]
data.find_all{|x| x > 5}.uniq
=> [6, 7]

Thats just the same as:

data.find_all do |x|
x > 5
end.uniq
=> [6, 7]

I do tend to use the {…} syntax if I’m chaining calls like this
though because ‘calling’ methods on end looks weird to me as well.

Alex G.

Bioinformatics Center
Kyoto University

simonsaid · September 7, 2007, 5:50am

putting it into phrase-test.rb and running it that way I get:

ph-test.rb:3: undefined method delete_at' for nil:NilClass (NoMethodError) from ph-test.rb:10:ineach_with_index’
from ph-test.rb:1:in each' from ph-test.rb:1:ineach_with_index’
from ph-test.rb:1

and relatedly, going through step by step I get:

023:0> end.delete_at(-1)
SyntaxError: compile error
(irb):23: syntax error, unexpected kEND
end.delete_at(-1)

I’ve never seen tacking things onto end before! is this possible?

simonsaid · September 7, 2007, 5:52am

eek. I’ve never written a unit test before. that’s all incredibly
murky territory… not only am I new to ruby, but new to programming in
general. I’ll get around to it someday, I think. it just seems scary.

simonsaid · September 7, 2007, 6:17am

On 7 Sep 2007, at 09:29, Simon S. wrote:

f.each_with_index { |x,y| # deleting all but the last (which is
}
f.delete_at(-1) # random whitespace item at the end from
the last quote

puts f[rand(f.size)]

The main problem is that you loop through the Array four separate
times first to chop, then to remove empty lines and then twice to
remove comments. There is no reason why you can’t combine them.
Perhaps this is a good example to learn how to use ‘or’, ‘and’ and
‘not’:

f = []

IO.readlines(“phrases_no_extra.txt”).each do |l|
l.chop!
f << l if l.size != 0 and not (l.include?("]") or l.include?("["))
end

puts f[rand(f.size)]

Alex G.

Bioinformatics Center
Kyoto University

simonsaid · September 7, 2007, 6:41am

quoth the Simon S.:

eek. I’ve never written a unit test before. that’s all incredibly
murky territory… not only am I new to ruby, but new to programming in
general. I’ll get around to it someday, I think. it just seems scary.

I used to think that too, but finally I got around to trying them, and
they
really aren’t that difficult at all. You basically just call functions
which
compare known, good values against the values your code generates. The
various canned functions allow you to test almost anything your code
ought to
be doing. The Pickaxe book has a great introduction that lays it out
simply
and succinctly. You owe it to yourself to check it out…[0]

As I discovered, when you start writing libraries or apps of any
appreciable
size the tests become almost indispensable. You will know immediately if
any
changes you made broke something anywhere in the code.

I have heard of folks who write unit tests directly from their
application’s
specs before they even start on code for the app itself. Then, it’s
simply a
matter of making tests pass, and when they all pass, your done!
Truthfully,
that’s what I find murky and scary

-d

[0] Sorry, was going to link to it but it seems that the chapter on unit
tests
was added in the second ed. Guess you will have to buy it … it’s worth
it,
trust me

simonsaid · September 7, 2007, 6:25am

On Sep 6, 5:29 pm, “Simon S.” [email protected] wrote:

files maybe. certain sections are almost whole sentences, they’re
f.delete_at(y)
if x.include? “[”
f.delete_at(y)
end
}
f.delete_at(-1) # random whitespace item at the end from the last quote

puts f[rand(f.size)]

While this does not handle everything in the input stream you mention,
This will print most of the lines you are looking for. This will skip
blank lines and lines with whose only content is a single upper case
letter. This will also remove the full and partial comments following
a phrase. With some work on the regular expressions it could probably
do what you want. This solution has the advantages of not reading the
entire input stream into memory before processing and of being
concise. It has the disadvantage of requiring Rio (http://
rio.rubyforge.org) which is not part of the standard ruby library.

require ‘rio’

rio(‘phrases_no_extra.txt’).chomp.lines(/^\S/).skip(/^[A-Z]?$/) do |
line|
puts line.gsub(/\s+[.*$/,‘’)
end

simonsaid · September 7, 2007, 7:22am

Ok, I’m going to try break this down a bit more than the other answers.
Simon S. wrote:

f = File.read(“phrases_no_extra.txt”)
f = f.to_a

f = IO.readlines(“phrases_no_extra.txt”)

f = f.each { |x| x.chop! }

Your “f =” at the start of this line does nothing. You’d be better off
with either:

 f.each {|x| x.chop! }

or I prefer:

 f.map! {|x| x.chop }

f.each_with_index { |x,y| # deletes the empty array items
if x.size == 0
f.delete_at(y)
end
}

For starters, I don’t think each_with_index will be stable if you delete
elements in the middle of the block, so this probably won’t work the way
you want it to.

That and there’s a convenient predicate for testing emptiness of
strings.

Much better would be:

  f.delete_if {|x| x.empty? }

f.each_with_index { |x,y| # deleting all but the last (which is
spread of two lines)
if x.include? “]” # of his comments
f.delete_at(y)
end
}

 f.delete_if {|x| x.include("]") }

f.each_with_index { |x,y| # yes, this is me unable to recall
how to do “or” hahaha.
if x.include? “[”
f.delete_at(y)
end
}

 f.delete_if {|x| x.include("[") }

f.delete_at(-1) # random whitespace item at the end from the last quote

 f.pop

puts f[rand(f.size)]

Nothing wrong with this line.

So, putting that all together and cleaning up a bit:

 f = IO.readlines("phrases_no_extra.txt").map {|x| x.chop }
 f.delete_if {|x| x.empty? || x.include?("[") || x.include?("]") }
 f.pop
 puts f[rand(f.size)]

That’s pretty neat I think, but if you agree with Alex that looping over
the array multiple times is a performance problem, then you should try
something like:

 f = []
 IO.each("phrases_no_extra.txt") do |x|
     x.chop!
     f << x unless x.empty? || x.include?("[") || x.include?("]")
 end
 f.pop
 puts f[rand(f.size)]

(This is an improvement over Alex’s solution in that it doesn’t read the
entire file into memory at once. Rather it steps over the file reading a
line at a time and only adding the ones you need to your array.)

Regards,

Pete Y.

simonsaid · September 7, 2007, 10:31am

On Sep 6, 7:29 pm, “Simon S.” [email protected] wrote:

files maybe. certain sections are almost whole sentences, they’re
f.delete_at(y)
if x.include? “[”
f.delete_at(y)
end
}
f.delete_at(-1) # random whitespace item at the end from the last quote

puts f[rand(f.size)]

a = IO.read(“phrases_no_extra.txt”).
split( /[\r?\n]+/ ).
reject{|s|
# Reject if length is less than 2 or if it contains
# a [ or if it’s composed entirely of upper case
# letters and spaces.
s.size < 2 or s.index “[” or s =~ /^[A-Z ]+$/ }

puts a[ rand( a.size ) ]

simonsaid · September 7, 2007, 9:46am

From: Simon S. [mailto:[email protected]]

anyhow, the sloppy newbie code is as follows:

be gentle to yourself. it is not sloppy. in fact i find it readable and
it clearly shows what the code is trying to do (wc may not be what you
want to do)… you can refactor it and put some tests as you go along.

f = File.read(“phrases_no_extra.txt”)

on my case, i’d prefer that my programs do *not carry baggage of fixed
static file names this way, my program does not depend on that
particular file, and i can use any file i want. try ARGF.

f = f.to_a

you can combine that w the above like

f = File.read(“phrases_no_extra.txt”).to_a

or you can use readlines to accomplish both, like,

f = File.readlines(“phrases_no_extra.txt”)

now, your original code makes many passes to array f. you can do it one
pass, and since delete_if accepts a block, you can do it like,

f.delete_if do |x|
x.sub!(/[./,“”) # remove trailing comments
x.sub!(/.].*/,“”) # remove leading comments, they span multi lines
x.strip! # remove leading/trailing whitespace
x.empty?
end

note, we are also modifying the array as we move along…

thus combining w the above codes, you can futher condense it to

f = File.readlines(“phrases_no_extra.txt”).delete_if do |x|
x.sub!(/[./,“”) # remove trailing comments
x.sub!(/.].*/,“”) # remove leading comments, they span multi
lines
x.strip! # remove leading/trailing whitespace
x.empty?
end

puts f[rand(f.size)]

you don’t need the delete_at(-1) since (you said) it’s a whitespace and
will be taken care of by the code block.

using ARGF, you save chars, and your program is flexible, like

f = ARGF.to_a.delete_if do |x|
x.sub!(/[./,“”) # remove trailing comments
x.sub!(/.].*/,“”) # remove leading comments
x.strip! # remove leading/trailing whitespace
x.empty?
end
puts f[rand(f.size)]

or, if you have fear of those ! methods (like me), you can do

f = ARGF.to_a.delete_if do |x|
x.replace x.sub(/[./,“”).sub(/.].*/,“”).strip
x.empty?
end
puts f[rand(f.size)]

kind regards -botp

simonsaid · September 7, 2007, 10:36am

[email protected] wrote:

appreciated though! I’m thinking of separating it into different text
if x.size == 0
how to do “or” hahaha.
reject{|s|
# Reject if length is less than 2 or if it contains
# a [ or if it’s composed entirely of upper case
# letters and spaces.
s.size < 2 or s.index “[” or s =~ /^[A-Z ]+$/ }

puts a[ rand( a.size ) ]

That contained some redundancy.

a = IO.read(“phrases_no_extra.txt”).
split( /[\r?\n]+/ ).
reject{|s|
# Reject if it contains a [ or if
# it’s composed entirely of upper case
# letters and spaces.
s.index “[” or s =~ /^[A-Z ]+$/ }

puts a[ rand( a.size ) ]

simonsaid · September 7, 2007, 3:07pm

John M. wrote:

On 9/7/07, Lloyd L. [email protected] wrote:

Jeremy W. wrote:

end.delete_at(-1)

OMG! You can call methods in an end??? I have lived far to long in the
scalar world! When would you do such a thing?

irb(main):004:0> [1, 2].collect do |i| i+3; end.join " : "
=> “4 : 5”

The method call is not on the “end” but on the result of the method
using the block that the “end”, well, ‘closes’. Very functional.

That is just too cool for words. Thanks for getting back to me!

simonsaid · September 7, 2007, 10:51am

On Sep 7, 3:34 am, [email protected] wrote:

that’s a little beyond me. any direction in this area would be kindly
f.each_with_index { |x,y| # deletes the empty array items
f.each_with_index { |x,y| # yes, this is me unable to recall
split( /[\r?\n]+/ ).
a = IO.read(“phrases_no_extra.txt”).
split( /[\r?\n]+/ ).
reject{|s|
# Reject if it contains a [ or if
# it's composed entirely of upper case
# letters and spaces.
s.index "["  or  s =~ /^[A-Z ]+$/ }
puts a[ rand( a.size ) ]

I wish that I were awake. That regular expression
wasn’t very sensible.

a = IO.read(“phrases_no_extra.txt”).
split( /(?:\r?\n)+/ ).
reject{|s|
# Reject if it contains a [ or if
# it’s composed entirely of upper case
# letters and spaces.
s.index “[” or s =~ /^[A-Z ]+$/ }

puts a[ rand( a.size ) ]

simonsaid · September 7, 2007, 9:57pm

Simon S. wrote:

putting it into phrase-test.rb and running it that way I get:

ph-test.rb:3: undefined method delete_at' for nil:NilClass (NoMethodError) from ph-test.rb:10:ineach_with_index’
from ph-test.rb:1:in each' from ph-test.rb:1:ineach_with_index’
from ph-test.rb:1

and relatedly, going through step by step I get:

023:0> end.delete_at(-1)
SyntaxError: compile error
(irb):23: syntax error, unexpected kEND
end.delete_at(-1)

I’ve never seen tacking things onto end before! is this possible?

hmm, well I guess that means that the delete_at(-1) isn’t being called
on an array, so the return value of that isn’t what I expected. But, to
answer the question it is possible to tack things on the end like that.
Take that delete_at(-1) off then run it and call f.class, it probably
says NilClass, but in the case it says Array, then I don’t know back to
the books for me

~Jeremy

simonsaid · September 8, 2007, 6:02pm

On 9/6/07, Simon S. [email protected] wrote:

eek. I’ve never written a unit test before. that’s all incredibly
murky territory… not only am I new to ruby, but new to programming in
general. I’ll get around to it someday, I think. it just seems scary.

Hah! Writing code that actually does stuff doesn’t seem scary to
you, but writing tests, that don’t even need to do anything but make
sure you code does the right thing, (and you don’t even have to write
the code first), is the scary, murky seeming thing? Writing code
without tests is murky. Writing code without tests is scary. Tests are
your friends.

simonsaid · September 7, 2007, 5:28pm

On Sep 6, 7:29 pm, “Simon S.” [email protected] wrote:

files maybe. certain sections are almost whole sentences, they’re
f.delete_at(y)
if x.include? “[”
f.delete_at(y)
end
}
f.delete_at(-1) # random whitespace item at the end from the last quote

puts f[rand(f.size)]

puts IO.readlines(“phrases_no_extra.txt”).
reject{|x| x =~ /[|^[A-Z ]*$/}.
instance_eval{ self[ rand(self.size) ] }