Help making a method more concise

[Mark Hayes]

[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

 

[Robert Klemme]

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

 

[Mark Hayes]

[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

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

--


Mark Hayes
(e-mail address removed)
<[email protected]>

 

[Josh Cheek]

[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
(e-mail address removed)
<[email protected]>

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

 

[Mark Hayes]

[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!

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 address removed)
<[email protected]>

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

--


Mark Hayes
(e-mail address removed)
<[email protected]>

 

[Christopher Dicely]

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

 

[Robert Klemme]

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?
[email protected]?
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/

 

[Christopher Dicely]

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?
[email protected]?
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.