New to ruby... is this bad code? Is there a more canon way to do this?

[Anthony Polito]

So I'm new to ruby and have writing some programs to play around with
it, writing some programs to solve various programing challenges out
there

One bit I've needed in several has been to choose n items from an array.

For a while I had a utility module that included a static method
choose(array, n) and returned an array

It worked just fine, but it bugged me, that seemed very non rubyish, so
I went with this...

## requires the size, <<, [], and []= operators
module Chooseable
def choose(n, &block)
hasBlock = block.nil?
c = clone
a = self.class.new
size.downto(size-n+1) do |x|
r = (x > 0) ? rand(x) : 0
a << (hasBlock ? c[r] : yield(c[r]))
temp = c[x-1]
c[x-1] = c[r]
c[r] = temp
end
return a
end
end

## Add a choose method to arrays. Is this bad ruby?
class Array
include Chooseable
end

--

At first I had a class ChooseableArray < Array because extending one of
the base ruby classes bothers me, but by just making array include
Chooseable I make my life so much easier. And because choose only
needs a few basic methods to work I can make say String include
Chooseable and suddenly I can choose a subset of characters from the
String class

Is sort of programing style reasonable Ruby? Am I missing a cleaner
way of doing things? Somehow the idea that I'm just sort of throwing
methods and modules into the core classes seems wrong. Or at least not
very scaleable.

 

[gabriele renzi]

Anthony Polito ha scritto:

So I'm new to ruby and have writing some programs to play around with
it, writing some programs to solve various programing challenges out there

One bit I've needed in several has been to choose n items from an array.

For a while I had a utility module that included a static method
choose(array, n) and returned an array

It worked just fine, but it bugged me, that seemed very non rubyish, so
I went with this...

just three quick things I notice:

- there is a method block_given? to delete hasBlock
- hasBlock uses a non rubyish naming convention (has_block)
usually you don't need a temp value to swap two things:
"c[x-1], c[r] = c[r], c[x-1]" should work

About the scalability isssue.. in general adding some method is
considered ok, while redefining has a bad feeling.
Anyway unit tests should happily catch problems when many programmer
change the same class.

Lastly, notice that #at_rand and #pick in the "facets" library may fit
with your problem

 

[Kenosis]

I don't believe the &block argument to the choose() method is really
necessary and I tend to think of rubyesque coding being minimalist
style-wise.

Also, isn't this line incorrect:

a << (hasBlock ? c[r] : yield(c[r]))

Shouldn't it be?

a << (hasBlock ? yield(c[r]) : c[r])

Ken


Ken

 

[Robert Klemme]

So I'm new to ruby and have writing some programs to play around with
it, writing some programs to solve various programing challenges out
there

One bit I've needed in several has been to choose n items from an
array.
For a while I had a utility module that included a static method
choose(array, n) and returned an array

It worked just fine, but it bugged me, that seemed very non rubyish,
so I went with this...

## requires the size, <<, [], and []= operators
module Chooseable
def choose(n, &block)
hasBlock = block.nil?
c = clone
a = self.class.new
size.downto(size-n+1) do |x|
r = (x > 0) ? rand(x) : 0
a << (hasBlock ? c[r] : yield(c[r]))
temp = c[x-1]
c[x-1] = c[r]
c[r] = temp
end
return a
end
end

## Add a choose method to arrays. Is this bad ruby?
class Array
include Chooseable
end

Do you have a usage example? It's not fully clear to me why you shuffle a
clone of the current instance and then discard it.

Kind regards

robert