Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > Ruby > Help making a method more concise

Reply
Thread Tools

Help making a method more concise

 
 
Mark Hayes
Guest
Posts: n/a
 
      05-02-2011
[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
http://www.velocityreviews.com/forums/(E-Mail Removed)
<(E-Mail Removed)>

 
Reply With Quote
 
 
 
 
Robert Klemme
Guest
Posts: n/a
 
      05-02-2011
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/
 
Reply With Quote
 
 
 
 
Mark Hayes
Guest
Posts: n/a
 
      05-02-2011
[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
<(E-Mail Removed)>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
(E-Mail Removed)
<(E-Mail Removed)>

 
Reply With Quote
 
Josh Cheek
Guest
Posts: n/a
 
      05-02-2011
[Note: parts of this message were removed to make it a legal post.]

On Mon, May 2, 2011 at 1:17 PM, Mark Hayes <(E-Mail Removed)> 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
> (E-Mail Removed)
> <(E-Mail Removed)>
>



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

 
Reply With Quote
 
Mark Hayes
Guest
Posts: n/a
 
      05-02-2011
[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 <(E-Mail Removed)> wrote:

> On Mon, May 2, 2011 at 1:17 PM, Mark Hayes <(E-Mail Removed)> 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
> > (E-Mail Removed)
> > <(E-Mail Removed)>
> >

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




--


Mark Hayes
(E-Mail Removed)
<(E-Mail Removed)>

 
Reply With Quote
 
Christopher Dicely
Guest
Posts: n/a
 
      05-03-2011
On Mon, May 2, 2011 at 11:17 AM, Mark Hayes <(E-Mail Removed)> 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

 
Reply With Quote
 
Robert Klemme
Guest
Posts: n/a
 
      05-03-2011
On Tue, May 3, 2011 at 6:28 AM, Christopher Dicely <(E-Mail Removed)> wro=
te:
> On Mon, May 2, 2011 at 11:17 AM, Mark Hayes <(E-Mail Removed)> 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?
> =(E-Mail Removed)?
> 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/

 
Reply With Quote
 
Christopher Dicely
Guest
Posts: n/a
 
      05-03-2011
On Tue, May 3, 2011 at 12:30 AM, Robert Klemme
<(E-Mail Removed)> wrote:
> On Tue, May 3, 2011 at 6:28 AM, Christopher Dicely <(E-Mail Removed)> w=

rote:
>> On Mon, May 2, 2011 at 11:17 AM, Mark Hayes <(E-Mail Removed)> 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=(E-Mail Removed)?
>> 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.

 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
write right method to be concise Pen Ttt Ruby 1 01-01-2011 08:10 PM
A More Concise Description of Numpy than the Guide to Numpy? W. eWatson Python 2 11-23-2009 08:58 PM
Re: More concise syntax for addressing std::vector* ??? Balog Pal C++ 15 03-23-2009 12:39 PM
More concise code barjunk Ruby 1 04-26-2007 05:23 AM
SQL: writing more concise paramaterized SQL darrel ASP .Net 13 03-30-2006 03:59 PM



Advertisments