Help making a method more concise

Discussion in 'Ruby' started by Mark Hayes, May 2, 2011.

  1. Mark Hayes

    Mark Hayes Guest

    [Note: parts of this message were removed to make it a legal post.]

    Hello,

    I'm looking to improve my skills as a Rubyist and would like to know if the
    "depth" method could be expressed more precisely. Any help would be greatly
    appreciated, thanks!

    require 'test/unit'

    class Node
    attr_accessor :value, :lchild, :rchild

    def depth
    [lchild ? lchild.depth : 0, rchild ? rchild.depth : 0].max + 1
    end
    end

    class NodeTest < Test::Unit::TestCase
    def setup
    @n = Node.new
    end

    def test_depth_of_one
    @n.lchild = Node.new
    @n.lchild.lchild = Node.new
    @n.rchild = Node.new
    assert @n.depth == 1, "depth of node should be one, was #{@n.depth}"
    end

    def test_depth_of_two
    @n.lchild = Node.new
    @n.rchild = Node.new
    assert @n.lchild.depth == 1, "depth of lchild should be one"
    assert @n.rchild.depth == 1, "depth of rchild should be one"
    assert @n.depth == 2, "depth of tree should be two, was #{@n.depth}"
    end
    end

    --

    Mark

    <>
     
    Mark Hayes, May 2, 2011
    #1
    1. Advertising

  2. On 02.05.2011 20:17, Mark Hayes wrote:
    > I'm looking to improve my skills as a Rubyist and would like to know if the
    > "depth" method could be expressed more precisely. Any help would be greatly
    > appreciated, thanks!
    >
    > require 'test/unit'
    >
    > class Node
    > attr_accessor :value, :lchild, :rchild
    >
    > def depth
    > [lchild ? lchild.depth : 0, rchild ? rchild.depth : 0].max + 1
    > end
    > end


    I think it's pretty OK that way. You could get rid of a bit of
    redundancy but whether that actually makes the program better? Judge
    for yourself:

    def depth
    [lchild, rchild].map {|ch| ch ? ch.depth : 0}.max + 1
    end

    def depth
    d = 0

    [lchild, rchild].each do |ch|
    d = ch.depth if ch && ch.depth > d
    end

    d + 1
    end

    def depth
    d = 0

    [lchild, rchild].each do |ch|
    d = [d, ch.depth].max if ch
    end

    d + 1
    end

    Kind regards

    robert

    --
    remember.guy do |as, often| as.you_can - without end
    http://blog.rubybestpractices.com/
     
    Robert Klemme, May 2, 2011
    #2
    1. Advertising

  3. Mark Hayes

    Mark Hayes Guest

    [Note: parts of this message were removed to make it a legal post.]

    Thanks Robert, I think I'll go with this one:

    def depth
    [lchild, rchild].map {|ch| ch ? ch.depth : 0}.max + 1
    end


    On Mon, May 2, 2011 at 11:30 AM, Robert Klemme
    <>wrote:

    > On 02.05.2011 20:17, Mark Hayes wrote:
    >
    >> I'm looking to improve my skills as a Rubyist and would like to know if
    >> the
    >> "depth" method could be expressed more precisely. Any help would be
    >> greatly
    >> appreciated, thanks!
    >>
    >> require 'test/unit'
    >>
    >> class Node
    >> attr_accessor :value, :lchild, :rchild
    >>
    >> def depth
    >> [lchild ? lchild.depth : 0, rchild ? rchild.depth : 0].max + 1
    >> end
    >> end
    >>

    >
    > I think it's pretty OK that way. You could get rid of a bit of redundancy
    > but whether that actually makes the program better? Judge for yourself:
    >
    > def depth
    > [lchild, rchild].map {|ch| ch ? ch.depth : 0}.max + 1
    > end
    >
    > def depth
    > d = 0
    >
    > [lchild, rchild].each do |ch|
    > d = ch.depth if ch && ch.depth > d
    > end
    >
    > d + 1
    > end
    >
    > def depth
    > d = 0
    >
    > [lchild, rchild].each do |ch|
    > d = [d, ch.depth].max if ch
    > end
    >
    > d + 1
    > end
    >
    > Kind regards
    >
    > robert
    >
    > --
    > remember.guy do |as, often| as.you_can - without end
    > http://blog.rubybestpractices.com/
    >
    >



    --


    Mark Hayes

    <>
     
    Mark Hayes, May 2, 2011
    #3
  4. Mark Hayes

    Josh Cheek Guest

    [Note: parts of this message were removed to make it a legal post.]

    On Mon, May 2, 2011 at 1:17 PM, Mark Hayes <> wrote:

    > Hello,
    >
    > I'm looking to improve my skills as a Rubyist and would like to know if the
    > "depth" method could be expressed more precisely. Any help would be
    > greatly
    > appreciated, thanks!
    >
    > require 'test/unit'
    >
    > class Node
    > attr_accessor :value, :lchild, :rchild
    >
    > def depth
    > [lchild ? lchild.depth : 0, rchild ? rchild.depth : 0].max + 1
    > end
    > end
    >
    > class NodeTest < Test::Unit::TestCase
    > def setup
    > @n = Node.new
    > end
    >
    > def test_depth_of_one
    > @n.lchild = Node.new
    > @n.lchild.lchild = Node.new
    > @n.rchild = Node.new
    > assert @n.depth == 1, "depth of node should be one, was #{@n.depth}"
    > end
    >
    > def test_depth_of_two
    > @n.lchild = Node.new
    > @n.rchild = Node.new
    > assert @n.lchild.depth == 1, "depth of lchild should be one"
    > assert @n.rchild.depth == 1, "depth of rchild should be one"
    > assert @n.depth == 2, "depth of tree should be two, was #{@n.depth}"
    > end
    > end
    >
    > --
    >
    > Mark
    >
    > <>
    >



    First test fails, with a value of 3.
    3 seems like the correct value, your test may be wrong.
     
    Josh Cheek, May 2, 2011
    #4
  5. Mark Hayes

    Mark Hayes Guest

    [Note: parts of this message were removed to make it a legal post.]

    Hey Josh,

    Oops ... I see that my test case was incorrect. It should've been:

    def test_depth_of_one
    @n.lchild = Node.new
    @n.lchild.lchild = Node.new
    @n.rchild = Node.new
    assert @n.depth == 3, "depth of node should be one, was #{@n.depth}"
    end

    Thanks for bringing that to my attention!

    On Mon, May 2, 2011 at 12:55 PM, Josh Cheek <> wrote:

    > On Mon, May 2, 2011 at 1:17 PM, Mark Hayes <> wrote:
    >
    > > Hello,
    > >
    > > I'm looking to improve my skills as a Rubyist and would like to know if

    > the
    > > "depth" method could be expressed more precisely. Any help would be
    > > greatly
    > > appreciated, thanks!
    > >
    > > require 'test/unit'
    > >
    > > class Node
    > > attr_accessor :value, :lchild, :rchild
    > >
    > > def depth
    > > [lchild ? lchild.depth : 0, rchild ? rchild.depth : 0].max + 1
    > > end
    > > end
    > >
    > > class NodeTest < Test::Unit::TestCase
    > > def setup
    > > @n = Node.new
    > > end
    > >
    > > def test_depth_of_one
    > > @n.lchild = Node.new
    > > @n.lchild.lchild = Node.new
    > > @n.rchild = Node.new
    > > assert @n.depth == 1, "depth of node should be one, was #{@n.depth}"
    > > end
    > >
    > > def test_depth_of_two
    > > @n.lchild = Node.new
    > > @n.rchild = Node.new
    > > assert @n.lchild.depth == 1, "depth of lchild should be one"
    > > assert @n.rchild.depth == 1, "depth of rchild should be one"
    > > assert @n.depth == 2, "depth of tree should be two, was #{@n.depth}"
    > > end
    > > end
    > >
    > > --
    > >
    > > Mark
    > >
    > > <>
    > >

    >
    >
    > First test fails, with a value of 3.
    > 3 seems like the correct value, your test may be wrong.
    >




    --


    Mark Hayes

    <>
     
    Mark Hayes, May 2, 2011
    #5
  6. On Mon, May 2, 2011 at 11:17 AM, Mark Hayes <> wrote:
    > Hello,
    >
    > I'm looking to improve my skills as a Rubyist and would like to know if t=

    he
    > "depth" method could be expressed more precisely. =C2=A0Any help would be=

    greatly
    > appreciated, thanks!
    >
    > require 'test/unit'
    >
    > class Node
    > =C2=A0attr_accessor :value, :lchild, :rchild
    >
    > =C2=A0def depth
    > =C2=A0 =C2=A0[lchild ? lchild.depth : 0, rchild ? rchild.depth : 0].max +=

    1
    > =C2=A0end
    > end



    def depth
    1 + [lchild,rchild].map {|ch| [ch}.max
    end

    or even:

    def depth
    1 + [lchild,rchild].map(&:to_i).max
    end

    alias to_i depth

    or:

    add empty (no value) left & right children when you first add a value
    to a node, and:

    def empty?
    @value.nil?
    end

    def depth
    empty? ? 0 : [lchild,rchild].map(&:depth).max
    end
     
    Christopher Dicely, May 3, 2011
    #6
  7. On Tue, May 3, 2011 at 6:28 AM, Christopher Dicely <> wro=
    te:
    > On Mon, May 2, 2011 at 11:17 AM, Mark Hayes <> wrote:
    >> Hello,
    >>
    >> I'm looking to improve my skills as a Rubyist and would like to know if =

    the
    >> "depth" method could be expressed more precisely. =A0Any help would be g=

    reatly
    >> appreciated, thanks!
    >>
    >> require 'test/unit'
    >>
    >> class Node
    >> =A0attr_accessor :value, :lchild, :rchild
    >>
    >> =A0def depth
    >> =A0 =A0[lchild ? lchild.depth : 0, rchild ? rchild.depth : 0].max + 1
    >> =A0end
    >> end

    >
    >
    > def depth
    > =A01 + [lchild,rchild].map {|ch| [ch}.max
    > end


    Does not work: apart from the syntax invocation of #depth is missing.
    If we add that we have a solution that has been proposed already.

    > or even:
    >
    > def depth
    > =A0 1 + [lchild,rchild].map(&:to_i).max
    > end
    >
    > alias to_i depth


    Now that's an interesting idea to use the knowledge that nil.to_i =3D> 0!

    > or:
    >
    > add empty (no value) left & right children when you first add a value
    > to a node, and:
    >
    > def empty?
    > =?
    > end
    >
    > def depth
    > =A0empty? ? 0 : [lchild,rchild].map(&:depth).max
    > end


    I don't think this captures the original semantics properly. Now
    there are only two states: empty, not empty. But the original design
    allowed for more states: empty, left set, right set, both set. Even
    if not for #depth this is likely important for other tree algorithms.

    Kind regards

    robert

    --=20
    remember.guy do |as, often| as.you_can - without end
    http://blog.rubybestpractices.com/
     
    Robert Klemme, May 3, 2011
    #7
  8. On Tue, May 3, 2011 at 12:30 AM, Robert Klemme
    <> wrote:
    > On Tue, May 3, 2011 at 6:28 AM, Christopher Dicely <> w=

    rote:
    >> On Mon, May 2, 2011 at 11:17 AM, Mark Hayes <> wrote:
    >>> Hello,
    >>>
    >>> I'm looking to improve my skills as a Rubyist and would like to know if=

    the
    >>> "depth" method could be expressed more precisely. =C2=A0Any help would =

    be greatly
    >>> appreciated, thanks!
    >>>
    >>> require 'test/unit'
    >>>
    >>> class Node
    >>> =C2=A0attr_accessor :value, :lchild, :rchild
    >>>
    >>> =C2=A0def depth
    >>> =C2=A0 =C2=A0[lchild ? lchild.depth : 0, rchild ? rchild.depth : 0].max=

    + 1
    >>> =C2=A0end
    >>> end

    >>
    >>
    >> def depth
    >> =C2=A01 + [lchild,rchild].map {|ch| [ch}.max
    >> end

    >
    > Does not work: apart from the syntax invocation of #depth is missing.
    > If we add that we have a solution that has been proposed already.


    Uh, yeah. I messed something up in editing that. I'm not entirely sure
    what it was supposed to be.

    >
    >> or even:
    >>
    >> def depth
    >> =C2=A0 1 + [lchild,rchild].map(&:to_i).max
    >> end
    >>
    >> alias to_i depth

    >
    > Now that's an interesting idea to use the knowledge that nil.to_i =3D> 0!
    >
    >> or:
    >>
    >> add empty (no value) left & right children when you first add a value
    >> to a node, and:
    >>
    >> def empty?
    >> =C2=?
    >> end
    >>
    >> def depth
    >> =C2=A0empty? ? 0 : [lchild,rchild].map(&:depth).max
    >> end

    >
    > I don't think this captures the original semantics properly. =C2=A0Now
    > there are only two states: empty, not empty. =C2=A0But the original desig=

    n
    > allowed for more states: empty, left set, right set, both set. =C2=A0Even
    > if not for #depth this is likely important for other tree algorithms.


    Yeah, it depends on things in the code we can't see; most of the tree
    implementations I've seen either use nil pointers for children and
    populate them with real nodes when they get data for them, or use
    empty nodes for children only of populated nodes and then populate the
    empty nodes when data arrives for them, but its possible that this one
    needs more differentiation.
     
    Christopher Dicely, May 3, 2011
    #8
    1. Advertising

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

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. darrel
    Replies:
    13
    Views:
    805
    darrel
    Mar 30, 2006
  2. Balog Pal
    Replies:
    15
    Views:
    544
    Anand Hariharan
    Mar 23, 2009
  3. W. eWatson
    Replies:
    2
    Views:
    1,047
    W. eWatson
    Nov 23, 2009
  4. barjunk

    More concise code

    barjunk, Apr 26, 2007, in forum: Ruby
    Replies:
    1
    Views:
    139
    Alex Gutteridge
    Apr 26, 2007
  5. Pen Ttt
    Replies:
    1
    Views:
    127
    Brian Candler
    Jan 1, 2011
Loading...

Share This Page