0
\$\begingroup\$

I've made a Ruby linked list data structure class and a lookup method. I know that lookup will only fetch the first occurrence and I'm ok with that.

Is it a bad implementation? Why is it bad? What can I improve to make it more idiomatic?

# linear collection of data elements, in which linear order is not given by
# their physical placement in memory. Instead, each element points to the next.
class LinkedList
  attr_accessor :value, :next

  def lookup(lookup_value)
    return self if value.eql? lookup_value
    return self.next.lookup(lookup_value) unless self.next.nil?
  end
end

Test code

root = LinkedList.new
root.value = 5

node1 = LinkedList.new
node1.value = 10

node2 = LinkedList.new
node2.value = 15

node3 = LinkedList.new
node3.value = 12

root.next = node1
node1.next = node2
node2.next = node3

p root.lookup(2) # nil
p root.lookup(15) # #<LinkedList:0x007f83db0a95b8 @value=15, @next=#<LinkedList:0x007f83db0a9590 @value=12>>
p root.lookup(12) # #<LinkedList:0x007fcbe91196a0 @value=12>
\$\endgroup\$
  • 1
    \$\begingroup\$ I would certainly make it easier to add elements (say node.addNew(value) and create some sort of constructor maybe following the pattern used by Hash[], i..e root = LinkedList[values, ...] \$\endgroup\$ – Marc Rohloff Dec 4 '17 at 18:32
1
\$\begingroup\$

Looks pretty good to me. You don't need the explicit return as the last statement of your lookup method. Also, in newer versions of Ruby (2.3+ I believe), you don't need the explicit nil check--instead, you can use the Safe Navigation Operator (&.).

  def lookup(lookup_value)
    return self if value.eql? lookup_value
    self.next&.lookup(lookup_value)
  end
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.