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

Discussion in 'Ruby' started by Anthony Polito, Apr 10, 2006.

  1. 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.
     
    Anthony Polito, Apr 10, 2006
    #1
    1. Advertisements

  2. Anthony Polito ha scritto:
    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 :)
     
    gabriele renzi, Apr 10, 2006
    #2
    1. Advertisements

  3. Anthony Polito

    Kenosis Guest

    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
     
    Kenosis, Apr 10, 2006
    #3
  4. 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
     
    Robert Klemme, Apr 11, 2006
    #4
    1. Advertisements

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments (here). After that, you can post your question and our members will help you out.